All of lore.kernel.org
 help / color / mirror / Atom feed
From: Louis Rilling <Louis.Rilling@kerlabs.com>
To: Joel Becker <Joel.Becker@oracle.com>
Cc: linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH][BUGFIX] configfs: Fix symlink() to a removing item
Date: Mon, 16 Jun 2008 20:09:11 +0200	[thread overview]
Message-ID: <20080616180911.GV30804@localhost> (raw)

Hi, the following patch fixes the symlink bug I mentioned a few days ago.
Thanks for your comments.

Louis

-- 
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: configfs-do-not-symlink-to-removing-item.patch
Type: text/x-diff
Size: 3385 bytes
Desc: not available
Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080616/468a2f8f/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080616/468a2f8f/attachment-0001.bin 

WARNING: multiple messages have this Message-ID (diff)
From: Louis Rilling <Louis.Rilling@kerlabs.com>
To: Joel Becker <Joel.Becker@oracle.com>
Cc: linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com
Subject: [PATCH][BUGFIX] configfs: Fix symlink() to a removing item
Date: Mon, 16 Jun 2008 20:09:11 +0200	[thread overview]
Message-ID: <20080616180911.GV30804@localhost> (raw)


[-- Attachment #1.1: Type: text/plain, Size: 292 bytes --]

Hi, the following patch fixes the symlink bug I mentioned a few days ago.
Thanks for your comments.

Louis

-- 
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes

[-- Attachment #1.2: configfs-do-not-symlink-to-removing-item.patch --]
[-- Type: text/x-diff, Size: 3470 bytes --]

configfs: Fix symlink() to a removing item

[Applies on top of rename() vs rmdir() deadlock fix patchset]

The rule for configfs symlinks is that symlinks always point to valid
config_items, and prevent the target from being removed. However,
configfs_symlink() only checks that it can grab a reference on the target item,
without ensuring that it remains alive until the symlink is correctly attached.

This patch makes configfs_symlink() fail whenever the target is being removed,
using the CONFIGFS_USET_DROPPING flag set by configfs_detach_prep() and
protected by configfs_dirent_lock.

This patch introduces a similar (weird?) behavior as with mkdir failures making
rmdir fail: if symlink() races with rmdir() of the parent directory (or its
youngest user-created ancestor if parent is a default group) or rmdir() of the
target directory, and then fails in configfs_create(), this can make the racing
rmdir() fail despite the concerned directory having no user-created entry (resp.
no symlink pointing to it or one of its default groups) in the end.
If this behavior is found unacceptable, I'll submit a fix in the same spirit as
the racing mkdir() fix.

Signed-off-by: Louis Rilling <Louis.Rilling@kerlabs.com>
---
 fs/configfs/dir.c     |   14 +++++++-------
 fs/configfs/symlink.c |    6 ++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

Index: b/fs/configfs/dir.c
===================================================================
--- a/fs/configfs/dir.c	2008-06-16 19:35:57.000000000 +0200
+++ b/fs/configfs/dir.c	2008-06-16 19:38:47.000000000 +0200
@@ -370,6 +370,9 @@ static int configfs_detach_prep(struct d
 	struct configfs_dirent *sd;
 	int ret;
 
+	/* Mark that we're trying to drop the group */
+	parent_sd->s_type |= CONFIGFS_USET_DROPPING;
+
 	ret = -EBUSY;
 	if (!list_empty(&parent_sd->s_links))
 		goto out;
@@ -385,8 +388,6 @@ static int configfs_detach_prep(struct d
 					*wait_mutex = &sd->s_dentry->d_inode->i_mutex;
 				return -EAGAIN;
 			}
-			/* Mark that we're trying to drop the group */
-			sd->s_type |= CONFIGFS_USET_DROPPING;
 
 			/*
 			 * Yup, recursive.  If there's a problem, blame
@@ -414,12 +415,11 @@ static void configfs_detach_rollback(str
 	struct configfs_dirent *parent_sd = dentry->d_fsdata;
 	struct configfs_dirent *sd;
 
-	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
-		if (sd->s_type & CONFIGFS_USET_DEFAULT) {
+	parent_sd->s_type &= ~CONFIGFS_USET_DROPPING;
+
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling)
+		if (sd->s_type & CONFIGFS_USET_DEFAULT)
 			configfs_detach_rollback(sd->s_dentry);
-			sd->s_type &= ~CONFIGFS_USET_DROPPING;
-		}
-	}
 }
 
 static void detach_attrs(struct config_item * item)
Index: b/fs/configfs/symlink.c
===================================================================
--- a/fs/configfs/symlink.c	2008-06-16 19:43:34.000000000 +0200
+++ b/fs/configfs/symlink.c	2008-06-16 19:47:06.000000000 +0200
@@ -78,6 +78,12 @@ static int create_link(struct config_ite
 	if (sl) {
 		sl->sl_target = config_item_get(item);
 		spin_lock(&configfs_dirent_lock);
+		if (target_sd->s_type & CONFIGFS_USET_DROPPING) {
+			spin_unlock(&configfs_dirent_lock);
+			config_item_put(item);
+			kfree(sl);
+			return -EPERM;
+		}
 		list_add(&sl->sl_list, &target_sd->s_links);
 		spin_unlock(&configfs_dirent_lock);
 		ret = configfs_create_link(sl, parent_item->ci_dentry,

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

             reply	other threads:[~2008-06-16 18:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-16 18:09 Louis Rilling [this message]
2008-06-16 18:09 ` [PATCH][BUGFIX] configfs: Fix symlink() to a removing item Louis Rilling
2008-06-16 22:29 ` [Ocfs2-devel] " Joel Becker
2008-06-16 22:29   ` Joel Becker
2008-06-17 10:42   ` [Ocfs2-devel] " Louis Rilling
2008-06-17 10:42     ` Louis Rilling
2008-06-17 12:56     ` [Ocfs2-devel] " Louis Rilling
2008-06-17 12:56       ` Louis Rilling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080616180911.GV30804@localhost \
    --to=louis.rilling@kerlabs.com \
    --cc=Joel.Becker@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ocfs2-devel@oss.oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.