diff for duplicates of <20080616180911.GV30804@localhost> diff --git a/N1/1.1.hdr b/N1/1.1.hdr new file mode 100644 index 0000000..5a32186 --- /dev/null +++ b/N1/1.1.hdr @@ -0,0 +1,3 @@ +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +Content-Transfer-Encoding: quoted-printable diff --git a/N1/1.1.txt b/N1/1.1.txt new file mode 100644 index 0000000..36cac35 --- /dev/null +++ b/N1/1.1.txt @@ -0,0 +1,10 @@ +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 diff --git a/N1/1.2.hdr b/N1/1.2.hdr new file mode 100644 index 0000000..e5ee0a5 --- /dev/null +++ b/N1/1.2.hdr @@ -0,0 +1,3 @@ +Content-Type: text/x-diff; charset=us-ascii +Content-Disposition: attachment; filename="configfs-do-not-symlink-to-removing-item.patch" +Content-Transfer-Encoding: quoted-printable diff --git a/N1/1.2.txt b/N1/1.2.txt new file mode 100644 index 0000000..744a9ea --- /dev/null +++ b/N1/1.2.txt @@ -0,0 +1,85 @@ +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, diff --git a/a/1.txt b/a/1.txt deleted file mode 100644 index 3330f89..0000000 --- a/a/1.txt +++ /dev/null @@ -1,24 +0,0 @@ -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 diff --git a/N1/2.bin b/N1/2.bin new file mode 100644 index 0000000..a204b7e --- /dev/null +++ b/N1/2.bin @@ -0,0 +1,7 @@ +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v1.4.6 (GNU/Linux) + +iD8DBQFIVqxHVKcRuvQ9Q1QRAjyOAKC0K/MGj+eIlcDQZzv8ioqBgS1AngCgpIdv +VCWfItbBJrqyAufhCogqLxc= +=zlxj +-----END PGP SIGNATURE----- diff --git a/N1/2.hdr b/N1/2.hdr new file mode 100644 index 0000000..764c300 --- /dev/null +++ b/N1/2.hdr @@ -0,0 +1,4 @@ +Content-Type: application/pgp-signature; name="signature.asc" +Content-Transfer-Encoding: 7bit +Content-Description: Digital signature +Content-Disposition: inline diff --git a/a/content_digest b/N1/content_digest index 723de83..c808182 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,10 +1,10 @@ "From\0Louis Rilling <Louis.Rilling@kerlabs.com>\0" - "Subject\0[Ocfs2-devel] [PATCH][BUGFIX] configfs: Fix symlink() to a removing item\0" + "Subject\0[PATCH][BUGFIX] configfs: Fix symlink() to a removing item\0" "Date\0Mon, 16 Jun 2008 20:09:11 +0200\0" "To\0Joel Becker <Joel.Becker@oracle.com>\0" "Cc\0linux-kernel@vger.kernel.org" " ocfs2-devel@oss.oracle.com\0" - "\00:1\0" + "\02:1.1\0" "b\0" "Hi, the following patch fixes the symlink bug I mentioned a few days ago.\n" "Thanks for your comments.\n" @@ -15,20 +15,105 @@ "Dr Louis Rilling\t\t\tKerlabs\n" "Skype: louis.rilling\t\t\tBatiment Germanium\n" "Phone: (+33|0) 6 80 89 08 23\t\t80 avenue des Buttes de Coesmes\n" - "http://www.kerlabs.com/\t\t\t35700 Rennes\n" - "-------------- next part --------------\n" - "A non-text attachment was scrubbed...\n" - "Name: configfs-do-not-symlink-to-removing-item.patch\n" - "Type: text/x-diff\n" - "Size: 3385 bytes\n" - "Desc: not available\n" - "Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080616/468a2f8f/attachment.bin \n" - "-------------- next part --------------\n" - "A non-text attachment was scrubbed...\n" - "Name: not available\n" - "Type: application/pgp-signature\n" - "Size: 189 bytes\n" - "Desc: Digital signature\n" - Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080616/468a2f8f/attachment-0001.bin + "http://www.kerlabs.com/\t\t\t35700 Rennes" + "\02:1.2\0" + "fn\0configfs-do-not-symlink-to-removing-item.patch\0" + "b\0" + "configfs: Fix symlink() to a removing item\n" + "\n" + "[Applies on top of rename() vs rmdir() deadlock fix patchset]\n" + "\n" + "The rule for configfs symlinks is that symlinks always point to valid\n" + "config_items, and prevent the target from being removed. However,\n" + "configfs_symlink() only checks that it can grab a reference on the target item,\n" + "without ensuring that it remains alive until the symlink is correctly attached.\n" + "\n" + "This patch makes configfs_symlink() fail whenever the target is being removed,\n" + "using the CONFIGFS_USET_DROPPING flag set by configfs_detach_prep() and\n" + "protected by configfs_dirent_lock.\n" + "\n" + "This patch introduces a similar (weird?) behavior as with mkdir failures making\n" + "rmdir fail: if symlink() races with rmdir() of the parent directory (or its\n" + "youngest user-created ancestor if parent is a default group) or rmdir() of the\n" + "target directory, and then fails in configfs_create(), this can make the racing\n" + "rmdir() fail despite the concerned directory having no user-created entry (resp.\n" + "no symlink pointing to it or one of its default groups) in the end.\n" + "If this behavior is found unacceptable, I'll submit a fix in the same spirit as\n" + "the racing mkdir() fix.\n" + "\n" + "Signed-off-by: Louis Rilling <Louis.Rilling@kerlabs.com>\n" + "---\n" + " fs/configfs/dir.c | 14 +++++++-------\n" + " fs/configfs/symlink.c | 6 ++++++\n" + " 2 files changed, 13 insertions(+), 7 deletions(-)\n" + "\n" + "Index: b/fs/configfs/dir.c\n" + "===================================================================\n" + "--- a/fs/configfs/dir.c\t2008-06-16 19:35:57.000000000 +0200\n" + "+++ b/fs/configfs/dir.c\t2008-06-16 19:38:47.000000000 +0200\n" + "@@ -370,6 +370,9 @@ static int configfs_detach_prep(struct d\n" + " \tstruct configfs_dirent *sd;\n" + " \tint ret;\n" + " \n" + "+\t/* Mark that we're trying to drop the group */\n" + "+\tparent_sd->s_type |= CONFIGFS_USET_DROPPING;\n" + "+\n" + " \tret = -EBUSY;\n" + " \tif (!list_empty(&parent_sd->s_links))\n" + " \t\tgoto out;\n" + "@@ -385,8 +388,6 @@ static int configfs_detach_prep(struct d\n" + " \t\t\t\t\t*wait_mutex = &sd->s_dentry->d_inode->i_mutex;\n" + " \t\t\t\treturn -EAGAIN;\n" + " \t\t\t}\n" + "-\t\t\t/* Mark that we're trying to drop the group */\n" + "-\t\t\tsd->s_type |= CONFIGFS_USET_DROPPING;\n" + " \n" + " \t\t\t/*\n" + " \t\t\t * Yup, recursive. If there's a problem, blame\n" + "@@ -414,12 +415,11 @@ static void configfs_detach_rollback(str\n" + " \tstruct configfs_dirent *parent_sd = dentry->d_fsdata;\n" + " \tstruct configfs_dirent *sd;\n" + " \n" + "-\tlist_for_each_entry(sd, &parent_sd->s_children, s_sibling) {\n" + "-\t\tif (sd->s_type & CONFIGFS_USET_DEFAULT) {\n" + "+\tparent_sd->s_type &= ~CONFIGFS_USET_DROPPING;\n" + "+\n" + "+\tlist_for_each_entry(sd, &parent_sd->s_children, s_sibling)\n" + "+\t\tif (sd->s_type & CONFIGFS_USET_DEFAULT)\n" + " \t\t\tconfigfs_detach_rollback(sd->s_dentry);\n" + "-\t\t\tsd->s_type &= ~CONFIGFS_USET_DROPPING;\n" + "-\t\t}\n" + "-\t}\n" + " }\n" + " \n" + " static void detach_attrs(struct config_item * item)\n" + "Index: b/fs/configfs/symlink.c\n" + "===================================================================\n" + "--- a/fs/configfs/symlink.c\t2008-06-16 19:43:34.000000000 +0200\n" + "+++ b/fs/configfs/symlink.c\t2008-06-16 19:47:06.000000000 +0200\n" + "@@ -78,6 +78,12 @@ static int create_link(struct config_ite\n" + " \tif (sl) {\n" + " \t\tsl->sl_target = config_item_get(item);\n" + " \t\tspin_lock(&configfs_dirent_lock);\n" + "+\t\tif (target_sd->s_type & CONFIGFS_USET_DROPPING) {\n" + "+\t\t\tspin_unlock(&configfs_dirent_lock);\n" + "+\t\t\tconfig_item_put(item);\n" + "+\t\t\tkfree(sl);\n" + "+\t\t\treturn -EPERM;\n" + "+\t\t}\n" + " \t\tlist_add(&sl->sl_list, &target_sd->s_links);\n" + " \t\tspin_unlock(&configfs_dirent_lock);\n" + " \t\tret = configfs_create_link(sl, parent_item->ci_dentry," + "\01:2\0" + "fn\0signature.asc\0" + "d\0Digital signature\0" + "b\0" + "-----BEGIN PGP SIGNATURE-----\n" + "Version: GnuPG v1.4.6 (GNU/Linux)\n" + "\n" + "iD8DBQFIVqxHVKcRuvQ9Q1QRAjyOAKC0K/MGj+eIlcDQZzv8ioqBgS1AngCgpIdv\n" + "VCWfItbBJrqyAufhCogqLxc=\n" + "=zlxj\n" + "-----END PGP SIGNATURE-----\n" -cd886c1f567b745460119cbf4553623aad9610af5167446a09eab3c902e9786b +0ba93d209c1b8b7cdc2da0568b04baca1787fb2387f47c633a5039f36861dc70
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.