[scan-admin@coverity.com: New Defects reported by Coverity Scan for Das U-Boot]
Here's the latest report from Coverity scan. I think it's more helpful to send these to the list so that anyone can help advise on solutions than to send it only to people that may have introduced the problem, as I have previously been forwarding to. I don't recall why I got in the habit to start with, so, breaking that habit now.
----- Forwarded message from scan-admin@coverity.com -----
Date: Wed, 28 Oct 2020 20:41:49 +0000 (UTC) From: scan-admin@coverity.com To: tom.rini@gmail.com Subject: New Defects reported by Coverity Scan for Das U-Boot
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
37 new defect(s) introduced to Das U-Boot found with Coverity Scan. 5 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 20 of 37 defect(s)
** CID 312960: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe()
________________________________________________________________________________________________________ *** CID 312960: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe() 101 mask = mux_reg_masks[2 * i + 1]; 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106
CID 312960: Integer handling issues (BAD_SHIFT) In expression "0xffffffffffffffffUL << field.lsb", left shifting by more than 63 bits has undefined behavior. The shift amount, "field.lsb", is 4294967295.
107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
** CID 312959: (RESOURCE_LEAK) /drivers/mux/mmio.c: 113 in mmio_mux_probe() /drivers/mux/mmio.c: 108 in mmio_mux_probe()
________________________________________________________________________________________________________ *** CID 312959: (RESOURCE_LEAK) /drivers/mux/mmio.c: 113 in mmio_mux_probe() 107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
CID 312959: (RESOURCE_LEAK) Variable "idle_states" going out of scope leaks the storage it points to.
113 return log_msg_ret("regmap_field_alloc", ret); 114 } 115 116 bits = 1 + field.msb - field.lsb; 117 mux->states = 1 << bits; 118 /drivers/mux/mmio.c: 108 in mmio_mux_probe() 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106 107 if (mask != GENMASK(field.msb, field.lsb))
CID 312959: (RESOURCE_LEAK) Variable "idle_states" going out of scope leaks the storage it points to.
108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]); 113 return log_msg_ret("regmap_field_alloc", ret);
** CID 312958: Uninitialized variables (UNINIT) /fs/btrfs/inode.c: 341 in btrfs_lookup_path()
________________________________________________________________________________________________________ *** CID 312958: Uninitialized variables (UNINIT) /fs/btrfs/inode.c: 341 in btrfs_lookup_path() 335 cur += len; 336 } 337 338 if (!ret) { 339 *root_ret = root; 340 *ino_ret = ino;
CID 312958: Uninitialized variables (UNINIT) Using uninitialized value "type".
341 *type_ret = type; 342 } 343 344 return ret; 345 } 346
** CID 312957: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /fs/btrfs/volumes.c: 1106 in __btrfs_map_block()
________________________________________________________________________________________________________ *** CID 312957: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /fs/btrfs/volumes.c: 1106 in __btrfs_map_block() 1100 stripe_nr = stripe_nr / nr_data_stripes(map); 1101 1102 /* Work out the disk rotation on this stripe-set */ 1103 rot = stripe_nr % map->num_stripes; 1104 1105 /* Fill in the logical address of each stripe */
CID 312957: Integer handling issues (OVERFLOW_BEFORE_WIDEN) Potentially overflowing expression "stripe_nr * nr_data_stripes(map)" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
1106 tmp = stripe_nr * nr_data_stripes(map); 1107 1108 for (i = 0; i < nr_data_stripes(map); i++) 1109 raid_map[(i+rot) % map->num_stripes] = 1110 ce->start + (tmp + i) * map->stripe_len; 1111
** CID 312956: Error handling issues (NEGATIVE_RETURNS) /tools/image-host.c: 337 in get_random_data()
________________________________________________________________________________________________________ *** CID 312956: Error handling issues (NEGATIVE_RETURNS) /tools/image-host.c: 337 in get_random_data() 331 ret = -1; 332 goto out; 333 } 334 335 ret = clock_gettime(CLOCK_MONOTONIC, &date); 336 if (ret < 0) {
CID 312956: Error handling issues (NEGATIVE_RETURNS) "ret" is passed to a parameter that cannot be negative.
337 printf("%s: clock_gettime has failed (err=%d, str=%s)\n", 338 __func__, ret, strerror(ret)); 339 goto out; 340 } 341 342 srand(date.tv_nsec);
** CID 312955: Uninitialized variables (UNINIT) /fs/btrfs/btrfs.c: 113 in show_dir()
________________________________________________________________________________________________________ *** CID 312955: Uninitialized variables (UNINIT) /fs/btrfs/btrfs.c: 113 in show_dir() 107 printf("%24.24s %.*s", filetime, btrfs_dir_name_len(eb, di), namebuf); 108 if (type == BTRFS_FT_SYMLINK) 109 printf(" -> %s", target ? target : "?"); 110 printf("\n"); 111 out: 112 free(target);
CID 312955: Uninitialized variables (UNINIT) Using uninitialized value "ret".
113 return ret; 114 } 115 116 int btrfs_probe(struct blk_desc *fs_dev_desc, 117 struct disk_partition *fs_partition) 118 {
** CID 312954: (DC.WEAK_CRYPTO) /test/dm/mux-cmd.c: 133 in dm_test_cmd_mux_select() /test/dm/mux-cmd.c: 129 in dm_test_cmd_mux_select()
________________________________________________________________________________________________________ *** CID 312954: (DC.WEAK_CRYPTO) /test/dm/mux-cmd.c: 133 in dm_test_cmd_mux_select() 127 ut_assertnonnull(chip); 128 129 srand(get_ticks() + rand()); 130 for (i = 0; i < chip->controllers; i++) { 131 mux = &chip->mux[i]; 132
CID 312954: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
133 state = rand() % mux->states; 134 135 snprintf(cmd, BUF_SIZE, "mux select a-mux-controller %x %x", i, 136 state); 137 run_command(cmd, 0); 138 ut_asserteq(!!mux->in_use, true); /test/dm/mux-cmd.c: 129 in dm_test_cmd_mux_select() 123 124 ut_assertok(uclass_get_device_by_name(UCLASS_MUX, "a-mux-controller", 125 &dev)); 126 chip = dev_get_uclass_priv(dev); 127 ut_assertnonnull(chip); 128
CID 312954: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
129 srand(get_ticks() + rand()); 130 for (i = 0; i < chip->controllers; i++) { 131 mux = &chip->mux[i]; 132 133 state = rand() % mux->states; 134
** CID 312953: Security best practices violations (DC.WEAK_CRYPTO) /tools/image-host.c: 345 in get_random_data()
________________________________________________________________________________________________________ *** CID 312953: Security best practices violations (DC.WEAK_CRYPTO) /tools/image-host.c: 345 in get_random_data() 339 goto out; 340 } 341 342 srand(date.tv_nsec); 343 344 for (i = 0; i < size; i++) {
CID 312953: Security best practices violations (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
345 *tmp = rand() & 0xff; 346 tmp++; 347 } 348 349 out: 350 return ret;
** CID 312952: Resource leaks (RESOURCE_LEAK) /drivers/reset/reset-uclass.c: 331 in devm_reset_bulk_get_by_node()
________________________________________________________________________________________________________ *** CID 312952: Resource leaks (RESOURCE_LEAK) /drivers/reset/reset-uclass.c: 331 in devm_reset_bulk_get_by_node() 325 __GFP_ZERO); 326 if (unlikely(!bulk)) 327 return ERR_PTR(-ENOMEM); 328 329 rc = __reset_get_bulk(dev, node, bulk); 330 if (rc)
CID 312952: Resource leaks (RESOURCE_LEAK) Variable "bulk" going out of scope leaks the storage it points to.
331 return ERR_PTR(rc); 332 333 devres_add(dev, bulk); 334 return bulk; 335 } 336
** CID 312951: (RESOURCE_LEAK) /drivers/core/regmap.c: 315 in devm_regmap_init() /drivers/core/regmap.c: 315 in devm_regmap_init() /drivers/core/regmap.c: 306 in devm_regmap_init() /drivers/core/regmap.c: 306 in devm_regmap_init()
________________________________________________________________________________________________________ *** CID 312951: (RESOURCE_LEAK) /drivers/core/regmap.c: 315 in devm_regmap_init() 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; 312 } 313 314 devres_add(dev, mapp);
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
315 return *mapp; 316 } 317 #endif 318 319 void *regmap_get_range(struct regmap *map, unsigned int range_num) 320 { /drivers/core/regmap.c: 315 in devm_regmap_init() 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; 312 } 313 314 devres_add(dev, mapp);
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
315 return *mapp; 316 } 317 #endif 318 319 void *regmap_get_range(struct regmap *map, unsigned int range_num) 320 { /drivers/core/regmap.c: 306 in devm_regmap_init() 300 if (config && config->r_size != 0) 301 rc = regmap_init_mem_range(dev_ofnode(dev), config->r_start, 302 config->r_size, mapp); 303 else 304 rc = regmap_init_mem(dev_ofnode(dev), mapp); 305 if (rc)
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
306 return ERR_PTR(rc); 307 308 map = *mapp; 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; /drivers/core/regmap.c: 306 in devm_regmap_init() 300 if (config && config->r_size != 0) 301 rc = regmap_init_mem_range(dev_ofnode(dev), config->r_start, 302 config->r_size, mapp); 303 else 304 rc = regmap_init_mem(dev_ofnode(dev), mapp); 305 if (rc)
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
306 return ERR_PTR(rc); 307 308 map = *mapp; 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift;
** CID 312950: Uninitialized variables (UNINIT)
________________________________________________________________________________________________________ *** CID 312950: Uninitialized variables (UNINIT) /fs/btrfs/btrfs.c: 96 in show_dir() 90 if (type < ARRAY_SIZE(dir_item_str) && dir_item_str[type]) 91 printf("<%s> ", dir_item_str[type]); 92 else 93 printf("DIR_ITEM.%u", type); 94 if (type == BTRFS_FT_CHRDEV || type == BTRFS_FT_BLKDEV) { 95 ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
CID 312950: Uninitialized variables (UNINIT) Using uninitialized value "ii.rdev" when calling "btrfs_stack_inode_rdev".
96 printf("%4llu,%5llu ", btrfs_stack_inode_rdev(&ii) >> 20, 97 btrfs_stack_inode_rdev(&ii) & 0xfffff); 98 } else { 99 if (key.type == BTRFS_INODE_ITEM_KEY) 100 printf("%10llu ", btrfs_stack_inode_size(&ii)); 101 else
** CID 312949: (DC.WEAK_CRYPTO) /test/dm/regmap.c: 310 in dm_test_devm_regmap() /test/dm/regmap.c: 308 in dm_test_devm_regmap()
________________________________________________________________________________________________________ *** CID 312949: (DC.WEAK_CRYPTO) /test/dm/regmap.c: 310 in dm_test_devm_regmap() 304 ut_assertok(uclass_get_device_by_name(UCLASS_NOP, "regmap-test_0", 305 &dev)); 306 priv = dev_get_priv(dev); 307 308 srand(get_ticks() + rand()); 309 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) {
CID 312949: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
310 pattern[i] = rand(); 311 ut_assertok(regmap_write(priv->cfg_regmap, i, pattern[i])); 312 } 313 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) { 314 ut_assertok(regmap_read(priv->cfg_regmap, i, &val)); 315 ut_asserteq(val, buffer[i]); /test/dm/regmap.c: 308 in dm_test_devm_regmap() 302 REGMAP_TEST_BUF_SZ * 2, MAP_NOCACHE); 303 304 ut_assertok(uclass_get_device_by_name(UCLASS_NOP, "regmap-test_0", 305 &dev)); 306 priv = dev_get_priv(dev); 307
CID 312949: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
308 srand(get_ticks() + rand()); 309 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) { 310 pattern[i] = rand(); 311 ut_assertok(regmap_write(priv->cfg_regmap, i, pattern[i])); 312 } 313 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) {
** CID 312948: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /fs/btrfs/volumes.c: 1033 in __btrfs_map_block()
________________________________________________________________________________________________________ *** CID 312948: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /fs/btrfs/volumes.c: 1033 in __btrfs_map_block() 1027 /* 1028 * stripe_nr counts the total number of stripes we have to stride 1029 * to get to this block 1030 */ 1031 stripe_nr = stripe_nr / map->stripe_len; 1032
CID 312948: Integer handling issues (OVERFLOW_BEFORE_WIDEN) Potentially overflowing expression "stripe_nr * map->stripe_len" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
1033 stripe_offset = stripe_nr * map->stripe_len; 1034 BUG_ON(offset < stripe_offset); 1035 1036 /* stripe_offset is the offset of this block in its stripe*/ 1037 stripe_offset = offset - stripe_offset; 1038
** CID 312947: Error handling issues (CHECKED_RETURN) /drivers/core/dump.c: 137 in dm_dump_drivers()
________________________________________________________________________________________________________ *** CID 312947: Error handling issues (CHECKED_RETURN) /drivers/core/dump.c: 137 in dm_dump_drivers() 131 int i; 132 133 puts("Driver uid uclass Devices\n"); 134 puts("----------------------------------------------------------\n"); 135 136 for (entry = d; entry < d + n_ents; entry++) {
CID 312947: Error handling issues (CHECKED_RETURN) Calling "uclass_get" without checking return value (as is done elsewhere 52 out of 65 times).
137 uclass_get(entry->id, &uc); 138 139 printf("%-25.25s %-3.3d %-20.20s ", entry->name, entry->id, 140 uc ? uc->uc_drv->name : "<no uclass>"); 141 142 if (!uc) {
** CID 312946: Incorrect expression (USELESS_CALL)
________________________________________________________________________________________________________ *** CID 312946: Incorrect expression (USELESS_CALL) /drivers/clk/clk-uclass.c: 201 in clk_set_default_get_by_id() 195 if (CONFIG_IS_ENABLED(CLK_CCF)) { 196 int ret = clk_get_by_id(clk->id, &c); 197 198 if (ret) { 199 debug("%s(): could not get parent clock pointer, id %lu\n", 200 __func__, clk->id);
CID 312946: Incorrect expression (USELESS_CALL) Calling "ERR_PTR(ret)" is only useful for its return value, which is ignored.
201 ERR_PTR(ret); 202 } 203 } 204 205 return c; 206 }
** CID 312945: Error handling issues (CHECKED_RETURN) /lib/efi_loader/efi_console.c: 272 in query_console_serial()
________________________________________________________________________________________________________ *** CID 312945: Error handling issues (CHECKED_RETURN) /lib/efi_loader/efi_console.c: 272 in query_console_serial() 266 { 267 int ret = 0; 268 int n[2]; 269 270 /* Empty input buffer */ 271 while (tstc())
CID 312945: Error handling issues (CHECKED_RETURN) Calling "getchar()" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
272 getchar(); 273 274 /* 275 * Not all terminals understand CSI [18t for querying the console size. 276 * We should adhere to escape sequences documented in the console_codes 277 * man page and the ECMA-48 standard.
** CID 312944: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe()
________________________________________________________________________________________________________ *** CID 312944: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe() 101 mask = mux_reg_masks[2 * i + 1]; 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106
CID 312944: Integer handling issues (BAD_SHIFT) In expression "0xffffffffffffffffUL >> 63U - field.msb", right shifting by more than 63 bits has undefined behavior. The shift amount, "63U - field.msb", is 64.
107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
** CID 312943: (TAINTED_SCALAR) /fs/btrfs/volumes.c: 563 in read_one_chunk() /fs/btrfs/volumes.c: 549 in read_one_chunk()
________________________________________________________________________________________________________ *** CID 312943: (TAINTED_SCALAR) /fs/btrfs/volumes.c: 563 in read_one_chunk() 557 map->io_align = btrfs_chunk_io_align(leaf, chunk); 558 map->sector_size = btrfs_chunk_sector_size(leaf, chunk); 559 map->stripe_len = btrfs_chunk_stripe_len(leaf, chunk); 560 map->type = btrfs_chunk_type(leaf, chunk); 561 map->sub_stripes = btrfs_chunk_sub_stripes(leaf, chunk); 562
CID 312943: (TAINTED_SCALAR) Using tainted variable "num_stripes" as a loop boundary.
563 for (i = 0; i < num_stripes; i++) { 564 map->stripes[i].physical = 565 btrfs_stripe_offset_nr(leaf, chunk, i); 566 devid = btrfs_stripe_devid_nr(leaf, chunk, i); 567 read_extent_buffer(leaf, uuid, (unsigned long) 568 btrfs_stripe_dev_uuid_nr(chunk, i), /fs/btrfs/volumes.c: 549 in read_one_chunk() 543 544 /* already mapped? */ 545 if (ce && ce->start <= logical && ce->start + ce->size > logical) { 546 return 0; 547 } 548
CID 312943: (TAINTED_SCALAR) Passing tainted variable "80UL + 16UL * num_stripes" to a tainted sink. [Note: The source code implementation of the function has been overridden by a builtin model.]
549 map = kmalloc(btrfs_map_lookup_size(num_stripes), GFP_NOFS); 550 if (!map) 551 return -ENOMEM; 552 553 map->ce.start = logical; 554 map->ce.size = length;
** CID 312942: Control flow issues (DEADCODE) /drivers/firmware/scmi/sandbox-scmi_devices.c: 96 in sandbox_scmi_devices_probe()
________________________________________________________________________________________________________ *** CID 312942: Control flow issues (DEADCODE) /drivers/firmware/scmi/sandbox-scmi_devices.c: 96 in sandbox_scmi_devices_probe() 90 } 91 92 return 0; 93 94 err_reset: 95 for (; n > 0; n--)
CID 312942: Control flow issues (DEADCODE) Execution cannot reach this statement: "reset_free(priv->devices.re...".
96 reset_free(priv->devices.reset + n - 1); 97 98 return ret; 99 } 100 101 static const struct udevice_id sandbox_scmi_devices_ids[] = {
** CID 312941: Insecure data handling (TAINTED_SCALAR) /fs/btrfs/dir-item.c: 57 in btrfs_match_dir_item_name()
________________________________________________________________________________________________________ *** CID 312941: Insecure data handling (TAINTED_SCALAR) /fs/btrfs/dir-item.c: 57 in btrfs_match_dir_item_name() 51 leaf = path->nodes[0]; 52 dir_item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_dir_item); 53 total_len = btrfs_item_size_nr(leaf, path->slots[0]); 54 if (verify_dir_item(root, leaf, dir_item)) 55 return NULL; 56
CID 312941: Insecure data handling (TAINTED_SCALAR) Using tainted variable "total_len" as a loop boundary.
57 while(cur < total_len) { 58 this_len = sizeof(*dir_item) + 59 btrfs_dir_name_len(leaf, dir_item) + 60 btrfs_dir_data_len(leaf, dir_item); 61 if (this_len > (total_len - cur)) { 62 fprintf(stderr, "invalid dir item size\n");
________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0...
To manage Coverity Scan email notifications for "tom.rini@gmail.com", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0...
----- End forwarded message -----
[Copy-pasting my reply to the off-list thread].
Hi,
On 30/10/20 10:45AM, Tom Rini wrote:
Hey all,
Here's the latest report from Coverity on new issues. Please take a look and let me know if any of these are false positives or things that we should try and adopt a Coverity model to cover. Thanks!
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Wed, Oct 28, 2020 at 4:41 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
37 new defect(s) introduced to Das U-Boot found with Coverity Scan. 5 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 20 of 37 defect(s)
** CID 312960: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe()
*** CID 312960: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe() 101 mask = mux_reg_masks[2 * i + 1]; 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106
CID 312960: Integer handling issues (BAD_SHIFT) In expression "0xffffffffffffffffUL << field.lsb", left shifting by more than 63 bits has undefined behavior. The shift amount, "field.lsb", is 4294967295.107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL);
Sounds like a legitimate complaint. If the mask is 0 then fls and ffs will return 0, and so msb and lsb will be 0xffffffff each. This will result in GENMASK() doing ~0UL << 0xffffffff. Of course, a mask of 0 is invalid but then this condition is supposed to check for invalid masks so that just defeats the purpose.
This code seems to check if a mask is all 1s or not. So it will catch a mask like 0b11101. But it will trip up on a mask like 0. My suggestion is to make the check something like:
if (ffs(mask) == 0 || mask != GENMASK(field.msb, field.lsb))
109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
** CID 312959: (RESOURCE_LEAK) /drivers/mux/mmio.c: 113 in mmio_mux_probe() /drivers/mux/mmio.c: 108 in mmio_mux_probe()
*** CID 312959: (RESOURCE_LEAK) /drivers/mux/mmio.c: 113 in mmio_mux_probe() 107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
CID 312959: (RESOURCE_LEAK) Variable "idle_states" going out of scope leaks the storage it points to.
Hmm... Not sure if this is actually a leak. idle_states is allocated using devm_kmalloc(), so if the probe fails the device should be destroyed, and idle_states with it. I'm not very well versed with managed APIs so maybe this is wrong. Dunno.
Anyway, idle_states is local to this function so I don't know if devm_kmalloc() is even needed. We might as well use regular kmalloc() because we free it at the end of probe anyway.
Any advice on this?
113 return log_msg_ret("regmap_field_alloc", ret); 114 } 115 116 bits = 1 + field.msb - field.lsb; 117 mux->states = 1 << bits; 118 /drivers/mux/mmio.c: 108 in mmio_mux_probe() 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106 107 if (mask != GENMASK(field.msb, field.lsb))
CID 312959: (RESOURCE_LEAK) Variable "idle_states" going out of scope leaks the storage it points to.
Same as above.
108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]); 113 return log_msg_ret("regmap_field_alloc", ret);
*** CID 312954: (DC.WEAK_CRYPTO) /test/dm/mux-cmd.c: 133 in dm_test_cmd_mux_select() 127 ut_assertnonnull(chip); 128 129 srand(get_ticks() + rand()); 130 for (i = 0; i < chip->controllers; i++) { 131 mux = &chip->mux[i]; 132
CID 312954: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
Not used for any security-related applications. No changes needed.
BTW, is the assertion that rand() is "linear congruential" even true for U-Boot's rand() or is it only true for the libc rand()?
133 state = rand() % mux->states; 134 135 snprintf(cmd, BUF_SIZE, "mux select a-mux-controller %x %x", i, 136 state); 137 run_command(cmd, 0); 138 ut_asserteq(!!mux->in_use, true); /test/dm/mux-cmd.c: 129 in dm_test_cmd_mux_select() 123 124 ut_assertok(uclass_get_device_by_name(UCLASS_MUX, "a-mux-controller", 125 &dev)); 126 chip = dev_get_uclass_priv(dev); 127 ut_assertnonnull(chip); 128
CID 312954: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
Same as above.
129 srand(get_ticks() + rand()); 130 for (i = 0; i < chip->controllers; i++) { 131 mux = &chip->mux[i]; 132 133 state = rand() % mux->states; 134
*** CID 312952: Resource leaks (RESOURCE_LEAK) /drivers/reset/reset-uclass.c: 331 in devm_reset_bulk_get_by_node() 325 __GFP_ZERO); 326 if (unlikely(!bulk)) 327 return ERR_PTR(-ENOMEM); 328 329 rc = __reset_get_bulk(dev, node, bulk); 330 if (rc)
CID 312952: Resource leaks (RESOURCE_LEAK) Variable "bulk" going out of scope leaks the storage it points to.
Similar problem as that of idle_states above. Not sure if memory allocated by devres_alloc() gets freed automatically but in this case I get the feeling it won't be.
331 return ERR_PTR(rc); 332 333 devres_add(dev, bulk); 334 return bulk; 335 } 336
** CID 312951: (RESOURCE_LEAK) /drivers/core/regmap.c: 315 in devm_regmap_init() /drivers/core/regmap.c: 315 in devm_regmap_init() /drivers/core/regmap.c: 306 in devm_regmap_init() /drivers/core/regmap.c: 306 in devm_regmap_init()
*** CID 312951: (RESOURCE_LEAK) /drivers/core/regmap.c: 315 in devm_regmap_init() 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; 312 } 313 314 devres_add(dev, mapp);
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
False positive because mapp is passed to devres_add().
315 return *mapp; 316 } 317 #endif 318 319 void *regmap_get_range(struct regmap *map, unsigned int range_num) 320 { /drivers/core/regmap.c: 315 in devm_regmap_init() 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; 312 } 313 314 devres_add(dev, mapp);
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
Same as above.
315 return *mapp; 316 } 317 #endif 318 319 void *regmap_get_range(struct regmap *map, unsigned int range_num) 320 { /drivers/core/regmap.c: 306 in devm_regmap_init() 300 if (config && config->r_size != 0) 301 rc = regmap_init_mem_range(dev_ofnode(dev), config->r_start, 302 config->r_size, mapp); 303 else 304 rc = regmap_init_mem(dev_ofnode(dev), mapp); 305 if (rc)
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
Hmm... We have not passed it to devres_add() yet. So this looks same as the problem with 'bulk' above. I think it is a leak but I would like someone to confirm my suspicion.
306 return ERR_PTR(rc); 307 308 map = *mapp; 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; /drivers/core/regmap.c: 306 in devm_regmap_init() 300 if (config && config->r_size != 0) 301 rc = regmap_init_mem_range(dev_ofnode(dev), config->r_start, 302 config->r_size, mapp); 303 else 304 rc = regmap_init_mem(dev_ofnode(dev), mapp); 305 if (rc)
CID 312951: (RESOURCE_LEAK) Variable "mapp" going out of scope leaks the storage it points to.
Same as above.
306 return ERR_PTR(rc); 307 308 map = *mapp; 309 if (config) { 310 map->width = config->width; 311 map->reg_offset_shift = config->reg_offset_shift; ________________________________________________________________________________________________________ *** CID 312949: (DC.WEAK_CRYPTO) /test/dm/regmap.c: 310 in dm_test_devm_regmap() 304 ut_assertok(uclass_get_device_by_name(UCLASS_NOP, "regmap-test_0", 305 &dev)); 306 priv = dev_get_priv(dev); 307 308 srand(get_ticks() + rand()); 309 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) {
CID 312949: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
False positive. Not used for any security-related applications.
310 pattern[i] = rand(); 311 ut_assertok(regmap_write(priv->cfg_regmap, i, pattern[i])); 312 } 313 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) { 314 ut_assertok(regmap_read(priv->cfg_regmap, i, &val)); 315 ut_asserteq(val, buffer[i]); /test/dm/regmap.c: 308 in dm_test_devm_regmap() 302 REGMAP_TEST_BUF_SZ * 2, MAP_NOCACHE); 303 304 ut_assertok(uclass_get_device_by_name(UCLASS_NOP, "regmap-test_0", 305 &dev)); 306 priv = dev_get_priv(dev); 307
CID 312949: (DC.WEAK_CRYPTO) "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.308 srand(get_ticks() + rand()); 309 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) { 310 pattern[i] = rand(); 311 ut_assertok(regmap_write(priv->cfg_regmap, i, pattern[i])); 312 } 313 for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) {
*** CID 312944: Integer handling issues (BAD_SHIFT) /drivers/mux/mmio.c: 107 in mmio_mux_probe() 101 mask = mux_reg_masks[2 * i + 1]; 102 103 field.reg = reg; 104 field.msb = fls(mask) - 1; 105 field.lsb = ffs(mask) - 1; 106
CID 312944: Integer handling issues (BAD_SHIFT) In expression "0xffffffffffffffffUL >> 63U - field.msb", right shifting by more than 63 bits has undefined behavior. The shift amount, "63U - field.msb", is 64.
Same problem as above. The tool should show issues with one file in sequence...
107 if (mask != GENMASK(field.msb, field.lsb)) 108 return log_msg_ret("invalid mask", -EINVAL); 109 110 fields[i] = devm_regmap_field_alloc(dev, regmap, field); 111 if (IS_ERR(fields[i])) { 112 ret = PTR_ERR(fields[i]);
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0...
To manage Coverity Scan email notifications for "tom.rini@gmail.com", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0...
Whew! That's a lot of issues with the patches I submitted! IMO the tool is mostly raising valid concerns and I think most of these are actual bugs.
I don't know how useful the rand() warning is though. I think it will be a false positive most of the time but maybe it is worth it for the one time it actually catches a security issue. Dunno.
participants (2)
-
Pratyush Yadav -
Tom Rini