[PATCH] Bsg referencing parent device

[Anatoliy Glagolev]

Bsg holding reference to a parent device may result in a crash
if user closes a bsg file handle after the parent device driver
has unloaded.
Holding a reference is not really needed: parent device must exist
between bsg_register_queue and bsg_unregister_queue. Before
the device goes away the caller does blk_cleanup_queue so that
all in-flight requests to the device are gone and all new requests
cannot pass beyond the queue. The queue itself is a refcounted
object and it will stay alive with a bsg file.
---
 block/bsg.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index defa06c..f9e4b91 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -650,18 +650,6 @@ static struct bsg_device *bsg_alloc_device(void)
 	return bd;
 }
 
-static void bsg_kref_release_function(struct kref *kref)
-{
-	struct bsg_class_device *bcd =
-		container_of(kref, struct bsg_class_device, ref);
-	struct device *parent = bcd->parent;
-
-	if (bcd->release)
-		bcd->release(bcd->parent);
-
-	put_device(parent);
-}
-
 static int bsg_put_device(struct bsg_device *bd)
 {
 	int ret = 0, do_free;
@@ -694,7 +682,6 @@ static int bsg_put_device(struct bsg_device *bd)
 
 	kfree(bd);
 out:
-	kref_put(&q->bsg_dev.ref, bsg_kref_release_function);
 	if (do_free)
 		blk_put_queue(q);
 	return ret;
@@ -760,8 +747,6 @@ static struct bsg_device *bsg_get_device(struct inode *inode, struct file *file)
 	 */
 	mutex_lock(&bsg_mutex);
 	bcd = idr_find(&bsg_minor_idr, iminor(inode));
-	if (bcd)
-		kref_get(&bcd->ref);
 	mutex_unlock(&bsg_mutex);
 
 	if (!bcd)
@@ -772,8 +757,6 @@ static struct bsg_device *bsg_get_device(struct inode *inode, struct file *file)
 		return bd;
 
 	bd = bsg_add_device(inode, bcd->queue, file);
-	if (IS_ERR(bd))
-		kref_put(&bcd->ref, bsg_kref_release_function);
 
 	return bd;
 }
@@ -902,6 +885,8 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 void bsg_unregister_queue(struct request_queue *q)
 {
+	struct device *parent;
+	void (*release)(struct device *dev);
 	struct bsg_class_device *bcd = &q->bsg_dev;
 
 	if (!bcd->class_dev)
@@ -913,8 +898,21 @@ void bsg_unregister_queue(struct request_queue *q)
 		sysfs_remove_link(&q->kobj, "bsg");
 	device_unregister(bcd->class_dev);
 	bcd->class_dev = NULL;
-	kref_put(&bcd->ref, bsg_kref_release_function);
+	parent = bcd->parent;
+	release = bcd->release;
+	bcd->parent = NULL;
+	bcd->release = NULL;
 	mutex_unlock(&bsg_mutex);
+
+	/*
+	 * The caller of bsg_[un]register_queue must hold a reference
+	 * to the parent device between ..register.. and ..unregister..
+	 * so we do not maintain a refcount to parent here.
+	 * Note: the caller is expected to blk_cleanup_queue(q)
+	 * before releasing the reference to the parent device.
+	 */
+	if (release)
+		release(parent);
 }
 EXPORT_SYMBOL_GPL(bsg_unregister_queue);
 
@@ -955,10 +953,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
 
 	bcd->minor = ret;
 	bcd->queue = q;
-	bcd->parent = get_device(parent);
+	bcd->parent = parent;
 	bcd->release = release;
 	bcd->ops = ops;
-	kref_init(&bcd->ref);
 	dev = MKDEV(bsg_major, bcd->minor);
 	class_dev = device_create(bsg_class, parent, dev, NULL, "%s", devname);
 	if (IS_ERR(class_dev)) {
-- 
1.9.1