* [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name
@ 2020-12-08 12:15 Dan Carpenter
2020-12-08 13:20 ` Haris Iqbal
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2020-12-08 12:15 UTC (permalink / raw)
To: haris.iqbal; +Cc: linux-block
Hello Md Haris Iqbal,
This is a semi-automatic email about new static checker warnings.
The patch 64e8a6ece1a5: "block/rnbd-clt: Dynamically alloc buffer for
pathname & blk_symlink_name" from Nov 26, 2020, leads to the
following Smatch complaint:
drivers/block/rnbd/rnbd-clt-sysfs.c:525 rnbd_clt_add_dev_symlink()
error: uninitialized symbol 'ret'.
drivers/block/rnbd/rnbd-clt-sysfs.c:524 rnbd_clt_add_dev_symlink()
error: we previously assumed 'dev->blk_symlink_name' could be null (see line 500)
drivers/block/rnbd/rnbd-clt-sysfs.c
493 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev)
494 {
495 struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj;
496 int ret, len;
497
498 len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2;
499 dev->blk_symlink_name = kzalloc(len, GFP_KERNEL);
500 if (!dev->blk_symlink_name) {
501 rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n");
502 goto out_err;
ret = -ENOMEM; here
503 }
504
505 ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name,
506 len);
507 if (ret) {
508 rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n",
509 ret);
510 goto out_err;
511 }
512
513 ret = sysfs_create_link(rnbd_devs_kobj, gd_kobj,
514 dev->blk_symlink_name);
515 if (ret) {
516 rnbd_clt_err(dev, "Creating /sys/block symlink failed, err: %d\n",
517 ret);
518 goto out_err;
519 }
520
521 return 0;
522
523 out_err:
524 dev->blk_symlink_name[0] = '\0';
^^^^^^^^^^^^^^^^^^^^^^^^
This will oops if the kzalloc() fails.
525 return ret;
526 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name 2020-12-08 12:15 [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name Dan Carpenter @ 2020-12-08 13:20 ` Haris Iqbal 2020-12-08 13:24 ` Jinpu Wang 0 siblings, 1 reply; 5+ messages in thread From: Haris Iqbal @ 2020-12-08 13:20 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-block, Danil Kipnis, Jinpu Wang On Tue, Dec 8, 2020 at 5:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > Hello Md Haris Iqbal, > > This is a semi-automatic email about new static checker warnings. > > The patch 64e8a6ece1a5: "block/rnbd-clt: Dynamically alloc buffer for > pathname & blk_symlink_name" from Nov 26, 2020, leads to the > following Smatch complaint: > > drivers/block/rnbd/rnbd-clt-sysfs.c:525 rnbd_clt_add_dev_symlink() > error: uninitialized symbol 'ret'. > > drivers/block/rnbd/rnbd-clt-sysfs.c:524 rnbd_clt_add_dev_symlink() > error: we previously assumed 'dev->blk_symlink_name' could be null (see line 500) > > drivers/block/rnbd/rnbd-clt-sysfs.c > 493 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev) > 494 { > 495 struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj; > 496 int ret, len; > 497 > 498 len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2; > 499 dev->blk_symlink_name = kzalloc(len, GFP_KERNEL); > 500 if (!dev->blk_symlink_name) { > 501 rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n"); > 502 goto out_err; > > ret = -ENOMEM; here > > 503 } > 504 > 505 ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name, > 506 len); > 507 if (ret) { > 508 rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n", > 509 ret); > 510 goto out_err; > 511 } > 512 > 513 ret = sysfs_create_link(rnbd_devs_kobj, gd_kobj, > 514 dev->blk_symlink_name); > 515 if (ret) { > 516 rnbd_clt_err(dev, "Creating /sys/block symlink failed, err: %d\n", > 517 ret); > 518 goto out_err; > 519 } > 520 > 521 return 0; > 522 > 523 out_err: > 524 dev->blk_symlink_name[0] = '\0'; > ^^^^^^^^^^^^^^^^^^^^^^^^ > This will oops if the kzalloc() fails. Thanks. Will send a patch soon. > > 525 return ret; > 526 } > > regards, > dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name 2020-12-08 13:20 ` Haris Iqbal @ 2020-12-08 13:24 ` Jinpu Wang 2020-12-08 14:05 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Jinpu Wang @ 2020-12-08 13:24 UTC (permalink / raw) To: Haris Iqbal, Dan Carpenter; +Cc: linux-block, Danil Kipnis Hi, On Tue, Dec 8, 2020 at 2:21 PM Haris Iqbal <haris.iqbal@cloud.ionos.com> wrote: > > On Tue, Dec 8, 2020 at 5:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > Hello Md Haris Iqbal, > > > > This is a semi-automatic email about new static checker warnings. > > > > The patch 64e8a6ece1a5: "block/rnbd-clt: Dynamically alloc buffer for > > pathname & blk_symlink_name" from Nov 26, 2020, leads to the > > following Smatch complaint: > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:525 rnbd_clt_add_dev_symlink() > > error: uninitialized symbol 'ret'. > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:524 rnbd_clt_add_dev_symlink() > > error: we previously assumed 'dev->blk_symlink_name' could be null (see line 500) > > > > drivers/block/rnbd/rnbd-clt-sysfs.c > > 493 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev) > > 494 { > > 495 struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj; > > 496 int ret, len; > > 497 > > 498 len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2; > > 499 dev->blk_symlink_name = kzalloc(len, GFP_KERNEL); > > 500 if (!dev->blk_symlink_name) { > > 501 rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n"); > > 502 goto out_err; > > > > ret = -ENOMEM; here > > > > 503 } > > 504 > > 505 ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name, > > 506 len); > > 507 if (ret) { > > 508 rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n", > > 509 ret); > > 510 goto out_err; > > 511 } > > 512 > > 513 ret = sysfs_create_link(rnbd_devs_kobj, gd_kobj, > > 514 dev->blk_symlink_name); > > 515 if (ret) { > > 516 rnbd_clt_err(dev, "Creating /sys/block symlink failed, err: %d\n", > > 517 ret); > > 518 goto out_err; > > 519 } > > 520 > > 521 return 0; > > 522 > > 523 out_err: > > 524 dev->blk_symlink_name[0] = '\0'; > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > This will oops if the kzalloc() fails. > > Thanks. Will send a patch soon. already fixed in block tree by Colin >https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.11/drivers&id=733c15bd3a944b8eeaacdddf061759b6a83dd3f4 > > > > 525 return ret; > > 526 } > > > > regards, > > dan carpenter Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name 2020-12-08 13:24 ` Jinpu Wang @ 2020-12-08 14:05 ` Dan Carpenter 2020-12-08 14:29 ` Jinpu Wang 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2020-12-08 14:05 UTC (permalink / raw) To: Jinpu Wang; +Cc: Haris Iqbal, linux-block, Danil Kipnis On Tue, Dec 08, 2020 at 02:24:40PM +0100, Jinpu Wang wrote: > Hi, > > On Tue, Dec 8, 2020 at 2:21 PM Haris Iqbal <haris.iqbal@cloud.ionos.com> wrote: > > > > On Tue, Dec 8, 2020 at 5:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > Hello Md Haris Iqbal, > > > > > > This is a semi-automatic email about new static checker warnings. > > > > > > The patch 64e8a6ece1a5: "block/rnbd-clt: Dynamically alloc buffer for > > > pathname & blk_symlink_name" from Nov 26, 2020, leads to the > > > following Smatch complaint: > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:525 rnbd_clt_add_dev_symlink() > > > error: uninitialized symbol 'ret'. > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:524 rnbd_clt_add_dev_symlink() > > > error: we previously assumed 'dev->blk_symlink_name' could be null (see line 500) > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c > > > 493 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev) > > > 494 { > > > 495 struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj; > > > 496 int ret, len; > > > 497 > > > 498 len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2; > > > 499 dev->blk_symlink_name = kzalloc(len, GFP_KERNEL); > > > 500 if (!dev->blk_symlink_name) { > > > 501 rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n"); > > > 502 goto out_err; > > > > > > ret = -ENOMEM; here > > > > > > 503 } > > > 504 > > > 505 ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name, > > > 506 len); > > > 507 if (ret) { > > > 508 rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n", > > > 509 ret); > > > 510 goto out_err; > > > 511 } > > > 512 > > > 513 ret = sysfs_create_link(rnbd_devs_kobj, gd_kobj, > > > 514 dev->blk_symlink_name); > > > 515 if (ret) { > > > 516 rnbd_clt_err(dev, "Creating /sys/block symlink failed, err: %d\n", > > > 517 ret); > > > 518 goto out_err; > > > 519 } > > > 520 > > > 521 return 0; > > > 522 > > > 523 out_err: > > > 524 dev->blk_symlink_name[0] = '\0'; > > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > > This will oops if the kzalloc() fails. > > > > Thanks. Will send a patch soon. > already fixed in block tree by Colin > >https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.11/drivers&id=733c15bd3a944b8eeaacdddf061759b6a83dd3f4 It's a weird thing that we don't free dev->blk_symlink_name if there is an error. It's impossible for rnbd_clt_get_path_name() to actually return an error in the current code, but if it did then only the last character of dev->blk_symlink_name[] is initialized. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name 2020-12-08 14:05 ` Dan Carpenter @ 2020-12-08 14:29 ` Jinpu Wang 0 siblings, 0 replies; 5+ messages in thread From: Jinpu Wang @ 2020-12-08 14:29 UTC (permalink / raw) To: Dan Carpenter; +Cc: Haris Iqbal, linux-block, Danil Kipnis On Tue, Dec 8, 2020 at 3:08 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Tue, Dec 08, 2020 at 02:24:40PM +0100, Jinpu Wang wrote: > > Hi, > > > > On Tue, Dec 8, 2020 at 2:21 PM Haris Iqbal <haris.iqbal@cloud.ionos.com> wrote: > > > > > > On Tue, Dec 8, 2020 at 5:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > > > Hello Md Haris Iqbal, > > > > > > > > This is a semi-automatic email about new static checker warnings. > > > > > > > > The patch 64e8a6ece1a5: "block/rnbd-clt: Dynamically alloc buffer for > > > > pathname & blk_symlink_name" from Nov 26, 2020, leads to the > > > > following Smatch complaint: > > > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:525 rnbd_clt_add_dev_symlink() > > > > error: uninitialized symbol 'ret'. > > > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c:524 rnbd_clt_add_dev_symlink() > > > > error: we previously assumed 'dev->blk_symlink_name' could be null (see line 500) > > > > > > > > drivers/block/rnbd/rnbd-clt-sysfs.c > > > > 493 static int rnbd_clt_add_dev_symlink(struct rnbd_clt_dev *dev) > > > > 494 { > > > > 495 struct kobject *gd_kobj = &disk_to_dev(dev->gd)->kobj; > > > > 496 int ret, len; > > > > 497 > > > > 498 len = strlen(dev->pathname) + strlen(dev->sess->sessname) + 2; > > > > 499 dev->blk_symlink_name = kzalloc(len, GFP_KERNEL); > > > > 500 if (!dev->blk_symlink_name) { > > > > 501 rnbd_clt_err(dev, "Failed to allocate memory for blk_symlink_name\n"); > > > > 502 goto out_err; > > > > > > > > ret = -ENOMEM; here > > > > > > > > 503 } > > > > 504 > > > > 505 ret = rnbd_clt_get_path_name(dev, dev->blk_symlink_name, > > > > 506 len); > > > > 507 if (ret) { > > > > 508 rnbd_clt_err(dev, "Failed to get /sys/block symlink path, err: %d\n", > > > > 509 ret); > > > > 510 goto out_err; > > > > 511 } > > > > 512 > > > > 513 ret = sysfs_create_link(rnbd_devs_kobj, gd_kobj, > > > > 514 dev->blk_symlink_name); > > > > 515 if (ret) { > > > > 516 rnbd_clt_err(dev, "Creating /sys/block symlink failed, err: %d\n", > > > > 517 ret); > > > > 518 goto out_err; > > > > 519 } > > > > 520 > > > > 521 return 0; > > > > 522 > > > > 523 out_err: > > > > 524 dev->blk_symlink_name[0] = '\0'; > > > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > > > This will oops if the kzalloc() fails. > > > > > > Thanks. Will send a patch soon. > > already fixed in block tree by Colin > > >https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.11/drivers&id=733c15bd3a944b8eeaacdddf061759b6a83dd3f4 > > It's a weird thing that we don't free dev->blk_symlink_name if there > is an error. It's impossible for rnbd_clt_get_path_name() to actually > return an error in the current code, but if it did then only the last > character of dev->blk_symlink_name[] is initialized. right, will send out a fix soon. > > regards, > dan carpenter > Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-08 14:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-08 12:15 [bug report] block/rnbd-clt: Dynamically alloc buffer for pathname & blk_symlink_name Dan Carpenter 2020-12-08 13:20 ` Haris Iqbal 2020-12-08 13:24 ` Jinpu Wang 2020-12-08 14:05 ` Dan Carpenter 2020-12-08 14:29 ` Jinpu Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox