[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1352736948-18243-1-git-send-email-dh.herrmann@googlemail.com>
Date: Mon, 12 Nov 2012 17:15:48 +0100
From: David Herrmann <dh.herrmann@...glemail.com>
To: fuse-devel@...ts.sourceforge.net
Cc: Miklos Szeredi <miklos@...redi.hu>, Tejun Heo <tj@...nel.org>,
linux-kernel@...r.kernel.org,
David Herrmann <dh.herrmann@...glemail.com>
Subject: [PATCH] cuse: do not register multiple devices with the same name
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@...glemail.com>
---
Hi
I am not sure whether this qualifies for the stable-tree, so please CC
stable@...r.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists