ayourtch / nat46

OpenWRT feed with stateless NAT46 kernel module
34 stars 28 forks source link

Fix coverity issues observed so far #30

Closed angus19 closed 2 years ago

angus19 commented 2 years ago

nat46-core: silence coverity warning of result_independent_of_operands nat46-module: avoid dereferencing a NULL pointer

angus19 commented 2 years ago

nat46-core.c

716 / Add the TCP/UDP pseudoheader, basing on the existing checksum / 717
718 sum16 csum_tcpudp_remagic(be32 saddr, __be32 daddr, unsigned short len, 719 unsigned char proto, u16 csum) { 720 u16 pdata; 721 u16 len0, len1; 722
723 pdata = (u16
)&saddr; 724 csum = csum16_upd(csum, 0, pdata++); 725 csum = csum16_upd(csum, 0, pdata++); 726 pdata = (u16 )&daddr; 727 csum = csum16_upd(csum, 0, pdata++); 728 csum = csum16_upd(csum, 0, *pdata++); 729
730 csum = csum16_upd(csum, 0, htons(proto)); (1) Event result_independent_of_operands: "len >> 16" is 0 regardless of the values of its operands. This occurs as the bitwise first operand of "&". 731 len1 = htons( (len >> 16) & 0xffff ); 732 len0 = htons(len & 0xffff); 733 csum = csum16_upd(csum, 0, len1); 734 csum = csum16_upd(csum, 0, len0); 735 return csum; 736 }

angus19 commented 2 years ago

nat46-module.c

85 static char *get_devname(char *ptail) 86 { 87 const int maxlen = IFNAMSIZ-1; (1) Event returned_null: "get_next_arg" returns "NULL" (checked 4 out of 5 times). [details] (8) Event var_assigned: Assigning: "devname" = "NULL" return value from "get_next_arg". Also see events: [example_assign][example_checked][example_checked][example_assign][example_checked][example_checked][dereference] 88 char devname = get_next_arg(ptail); (9) Event dereference: Dereferencing a pointer that might be "NULL" "devname" when calling "strlen". Also see events: [returned_null][example_assign][example_checked][example_checked][example_assign][example_checked][example_checked][var_assigned] 89 if(strlen(devname) > maxlen) { 90 printk(KERN_INFO "nat46: '%s' is " 91 "longer than %d chars, truncating\n", devname, maxlen); 92 devname[maxlen] = 0; 93 } 94 return devname; 95 } 96
97 static ssize_t nat46_proc_write(struct file file, const char __user buffer, 98 size_t count, loff_t ppos) 99 { 100 char buf = NULL; 101 char tail = NULL; 102 char devname = NULL; 103 char arg_name = NULL; 104
105 buf = kmalloc(sizeof(char)
(count + 1), GFP_KERNEL); 106 if (!buf) 107 return -ENOMEM; 108
109 if (copy_from_user(buf, buffer, count)) { 110 kfree(buf); 111 return -EFAULT; 112 } 113 tail = buf; 114 buf[count] = '\0'; 115 if( (count > 0) && (buf[count-1] == '\n') ) { 116 buf[count-1] = '\0'; 117 } 118
(7) Event example_checked: Example 4: "get_next_arg(&tail)" has its value checked in "NULL != (arg_name = get_next_arg(&tail))". Also see events: [returned_null][example_assign][example_checked][example_checked][example_assign][example_checked][var_assigned][dereference] 119 while (NULL != (arg_name = get_next_arg(&tail))) { 120 if (0 == strcmp(arg_name, "add")) { 121 devname = get_devname(&tail); 122 printk(KERN_INFO "nat46: adding device (%s)\n", devname); 123 mutex_lock(&add_del_lock); 124 nat46_create(devname); 125 mutex_unlock(&add_del_lock); 126 } else if (0 == strcmp(arg_name, "del")) { 127 devname = get_devname(&tail); 128 printk(KERN_INFO "nat46: deleting device (%s)\n", devname); 129 mutex_lock(&add_del_lock); 130 nat46_destroy(devname); 131 mutex_unlock(&add_del_lock); 132 } else if (0 == strcmp(arg_name, "config")) { 133 devname = get_devname(&tail); 134 printk(KERN_INFO "nat46: configure device (%s) with '%s'\n", devname, tail); 135 mutex_lock(&add_del_lock); 136 nat46_configure(devname, tail); 137 mutex_unlock(&add_del_lock); 138 } else if (0 == strcmp(arg_name, "insert")) { 139 devname = get_devname(&tail); 140 printk(KERN_INFO "nat46: insert new rule into device (%s) with '%s'\n", devname, tail); 141 mutex_lock(&add_del_lock); 142 nat46_insert(devname, tail); 143 mutex_unlock(&add_del_lock); 144 } else if (0 == strcmp(arg_name, "remove")) { 145 devname = get_devname(&tail); 146 printk(KERN_INFO "nat46: remove a rule from the device (%s) with '%s'\n", devname, tail); 147 mutex_lock(&add_del_lock); 148 nat46_remove(devname, tail); 149 mutex_unlock(&add_del_lock); 150 } 151 } 152
153 kfree(buf); 154 return count; 155 }