Re: [PATCH] scsi: bsg: Fix device unregistration

From: Zenghui Yu
Date: Fri Sep 10 2021 - 21:41:16 EST


On 2021/9/10 20:45, Johan Hovold wrote:
On Thu, Sep 09, 2021 at 11:46:08AM +0800, Zenghui Yu wrote:

@@ -218,6 +226,7 @@ struct bsg_device *bsg_register_queue(struct request_queue *q,
out_device_del:
cdev_device_del(&bd->cdev, &bd->device);
out_ida_remove:
+ put_device(&bd->device);
ida_simple_remove(&bsg_minor_ida, MINOR(bd->device.devt));
out_kfree:
kfree(bd);

Ehh, what about the blatant use-after-free and double-free you just
added here?

Yeah, whoops. That's definitely wrong. I'm squash the following fix
in this patch.

Thanks for the heads up!

|diff --git a/block/bsg.c b/block/bsg.c
|index c3bb92b9af7e..882f56bff14f 100644
|--- a/block/bsg.c
|+++ b/block/bsg.c
|@@ -200,7 +200,8 @@ struct bsg_device *bsg_register_queue(struct request_queue *q,
| if (ret < 0) {
| if (ret == -ENOSPC)
| dev_err(parent, "bsg: too many bsg devices\n");
|- goto out_kfree;
|+ kfree(bd);
|+ return ERR_PTR(ret);
| }
| bd->device.devt = MKDEV(bsg_major, ret);
| bd->device.class = bsg_class;
|@@ -213,7 +214,7 @@ struct bsg_device *bsg_register_queue(struct request_queue *q,
| bd->cdev.owner = THIS_MODULE;
| ret = cdev_device_add(&bd->cdev, &bd->device);
| if (ret)
|- goto out_ida_remove;
|+ goto out_put_device;
|
| if (q->kobj.sd) {
| ret = sysfs_create_link(&q->kobj, &bd->device.kobj, "bsg");
|@@ -225,11 +226,8 @@ struct bsg_device *bsg_register_queue(struct request_queue *q,
|
| out_device_del:
| cdev_device_del(&bd->cdev, &bd->device);
|-out_ida_remove:
|+out_put_device:
| put_device(&bd->device);
|- ida_simple_remove(&bsg_minor_ida, MINOR(bd->device.devt));
|-out_kfree:
|- kfree(bd);
| return ERR_PTR(ret);
| }
| EXPORT_SYMBOL_GPL(bsg_register_queue);