Messages in this thread Patch in this message | | | From | David Herrmann <> | Subject | [PATCH] cuse: do not register multiple devices with the same name | Date | Mon, 12 Nov 2012 17:15:48 +0100 |
| |
We do not check whether we already registered a CUSE device with a given name so we might end up with two devices with the same name. Sysfs will then complain as it cannot create suitable directories.
This patch makes the init-command fail if there is already a device with the given name. To avoid race-conditions during initialization, we actually need to add the device to the list while still holding the lock for the name-check. The new "ready" field guarantees that the device is still not opened until it is fully initialized.
Following the sysfs warnings when registering two devices with the same name:
------------[ cut here ]------------ WARNING: at fs/sysfs/dir.c:529 sysfs_add_one+0xc8/0xf0() Hardware name: N150P/N210P/N220P sysfs: cannot create duplicate filename '/devices/virtual/cuse/ttyFseat0' Modules linked in: btusb bluetooth Pid: 14089, comm: lt-kmscon Tainted: G W 3.5.3+ #60 Call Trace: [<ffffffff81136400>] ? sysfs_add_one+0x60/0xf0 [<ffffffff8102f99d>] warn_slowpath_common+0x7d/0xc0 [<ffffffff8102fa83>] warn_slowpath_fmt+0x43/0x50 [<ffffffff81136468>] sysfs_add_one+0xc8/0xf0 [<ffffffff81136686>] create_dir+0x76/0xd0 [<ffffffff81136a14>] sysfs_create_dir+0x84/0xe0 [<ffffffff811fe67b>] kobject_add_internal+0x9b/0x200 [<ffffffff811feb38>] kobject_add+0x68/0xc0 [<ffffffff81310c73>] device_add+0xe3/0x680 [<ffffffff8130f5ae>] ? dev_set_name+0x3e/0x40 [<ffffffff811c6834>] cuse_process_init_reply+0x204/0x410 [<ffffffff811c6630>] ? cuse_open+0xe0/0xe0 [<ffffffff811bb23c>] request_end+0xfc/0x1a0 [<ffffffff811bc6e2>] fuse_dev_do_write+0xa32/0xd10 [<ffffffff811ba435>] ? fuse_copy_one+0x45/0x60 [<ffffffff8109cf06>] ? find_get_page+0x66/0xb0 [<ffffffff811bccc0>] ? fuse_dev_splice_write+0x300/0x300 [<ffffffff811bcd29>] fuse_dev_write+0x69/0x80 [<ffffffff810d569c>] do_sync_readv_writev+0xdc/0x120 [<ffffffff810d57db>] ? rw_copy_check_uvector+0x6b/0x130 [<ffffffff810b9c5e>] ? handle_mm_fault+0x12e/0x1f0 [<ffffffff810d5973>] do_readv_writev+0xd3/0x1e0 [<ffffffff810d5ab0>] vfs_writev+0x30/0x60 [<ffffffff810d5c38>] sys_writev+0x48/0xb0 [<ffffffff815846a2>] system_call_fastpath+0x16/0x1b ---[ end trace 368eb04507b14c94 ]---
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com> --- Hi
I am not sure whether this qualifies for the stable-tree, so please CC stable@vger.kernel.org if you think so.
The patch is against linux-next from today.
Regards David
fs/fuse/cuse.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c index 1326051..11fbc52 100644 --- a/fs/fuse/cuse.c +++ b/fs/fuse/cuse.c @@ -33,6 +33,7 @@ * closed. */ +#include <linux/atomic.h> #include <linux/fuse.h> #include <linux/cdev.h> #include <linux/device.h> @@ -45,6 +46,7 @@ #include <linux/miscdevice.h> #include <linux/mutex.h> #include <linux/slab.h> +#include <linux/smp.h> #include <linux/spinlock.h> #include <linux/stat.h> #include <linux/module.h> @@ -54,6 +56,7 @@ #define CUSE_CONNTBL_LEN 64 struct cuse_conn { + atomic_t ready; /* device is ready for open() */ struct list_head list; /* linked on cuse_conntbl */ struct fuse_conn fc; /* fuse connection */ struct cdev *cdev; /* associated character device */ @@ -120,6 +123,10 @@ static int cuse_open(struct inode *inode, struct file *file) spin_lock(&cuse_lock); list_for_each_entry(pos, cuse_conntbl_head(devt), list) if (pos->dev->devt == devt) { + if (!atomic_read(&pos->ready)) + break; + smp_rmb(); + fuse_conn_get(&pos->fc); cc = pos; break; @@ -308,7 +315,7 @@ static void cuse_gendev_release(struct device *dev) */ static void cuse_process_init_reply(struct fuse_conn *fc, struct fuse_req *req) { - struct cuse_conn *cc = fc_to_cc(fc); + struct cuse_conn *cc = fc_to_cc(fc), *pos; struct cuse_init_out *arg = req->out.args[0].value; struct page *page = req->pages[0]; struct cuse_devinfo devinfo = { }; @@ -357,17 +364,30 @@ static void cuse_process_init_reply(struct fuse_conn *fc, struct fuse_req *req) dev->devt = devt; dev->release = cuse_gendev_release; dev_set_drvdata(dev, cc); + + /* make sure the device-name is unique */ + spin_lock(&cuse_lock); dev_set_name(dev, "%s", devinfo.name); + list_for_each_entry(pos, cuse_conntbl_head(devt), list) { + if (!strcmp(dev_name(pos->dev), dev_name(dev))) { + spin_unlock(&cuse_lock); + goto err_device; + } + } + + list_add(&cc->list, cuse_conntbl_head(devt)); + spin_unlock(&cuse_lock); + rc = device_add(dev); if (rc) - goto err_device; + goto err_list; /* register cdev */ rc = -ENOMEM; cdev = cdev_alloc(); if (!cdev) - goto err_device; + goto err_list; cdev->owner = THIS_MODULE; cdev->ops = &cuse_frontend_fops; @@ -380,9 +400,8 @@ static void cuse_process_init_reply(struct fuse_conn *fc, struct fuse_req *req) cc->cdev = cdev; /* make the device available */ - spin_lock(&cuse_lock); - list_add(&cc->list, cuse_conntbl_head(devt)); - spin_unlock(&cuse_lock); + smp_wmb(); + atomic_inc(&cc->ready); /* announce device availability */ dev_set_uevent_suppress(dev, 0); @@ -394,6 +413,10 @@ out: err_cdev: cdev_del(cdev); +err_list: + spin_lock(&cuse_lock); + list_del_init(&cc->list); + spin_unlock(&cuse_lock); err_device: put_device(dev); err_region: -- 1.8.0
| |