[patch 0/4] Reiserfs fixes

[patch 0/4] Reiserfs fixes

[Jeff Mahoney]

 Here are the remaining 4 patches for outstanding reiserfs bugs.

 Please apply.

 -Jeff

--
Jeff Mahoney
SUSE Labs

[patch 1/4] reiserfs: clean up ifdefs

[Jeff Mahoney]

[-- Attachment #1: patches.fixes/reiserfs-xattr-fixup --]
[-- Type: text/plain, Size: 2872 bytes --]

 With xattr cleanup even with xattrs disabled, much of the initial setup
 is still performed. Some #ifdefs are just not needed since the options
 they protect wouldn't be available anyway.

 This patch cleans those up.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/reiserfs/xattr.c |   49 ++++++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -685,20 +685,6 @@ out:
 	return err;
 }
 
-/* Actual operations that are exported to VFS-land */
-struct xattr_handler *reiserfs_xattr_handlers[] = {
-	&reiserfs_xattr_user_handler,
-	&reiserfs_xattr_trusted_handler,
-#ifdef CONFIG_REISERFS_FS_SECURITY
-	&reiserfs_xattr_security_handler,
-#endif
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
-	&reiserfs_posix_acl_access_handler,
-	&reiserfs_posix_acl_default_handler,
-#endif
-	NULL
-};
-
 /*
  * In order to implement different sets of xattr operations for each xattr
  * prefix with the generic xattr API, a filesystem should create a
@@ -922,6 +908,28 @@ static int create_privroot(struct dentry
 	return 0;
 }
 
+#else
+int __init reiserfs_xattr_register_handlers(void) { return 0; }
+void reiserfs_xattr_unregister_handlers(void) {}
+static int create_privroot(struct dentry *dentry) { return 0; }
+#endif
+
+/* Actual operations that are exported to VFS-land */
+struct xattr_handler *reiserfs_xattr_handlers[] = {
+#ifdef CONFIG_REISERFS_FS_XATTR
+	&reiserfs_xattr_user_handler,
+	&reiserfs_xattr_trusted_handler,
+#endif
+#ifdef CONFIG_REISERFS_FS_SECURITY
+	&reiserfs_xattr_security_handler,
+#endif
+#ifdef CONFIG_REISERFS_FS_POSIX_ACL
+	&reiserfs_posix_acl_access_handler,
+	&reiserfs_posix_acl_default_handler,
+#endif
+	NULL
+};
+
 static int xattr_mount_check(struct super_block *s)
 {
 	/* We need generation numbers to ensure that the oid mapping is correct
@@ -941,11 +949,6 @@ static int xattr_mount_check(struct supe
 	return 0;
 }
 
-#else
-int __init reiserfs_xattr_register_handlers(void) { return 0; }
-void reiserfs_xattr_unregister_handlers(void) {}
-#endif
-
 /* This will catch lookups from the fs root to .reiserfs_priv */
 static int
 xattr_lookup_poison(struct dentry *dentry, struct qstr *q1, struct qstr *name)
@@ -992,7 +995,6 @@ int reiserfs_xattr_init(struct super_blo
 	int err = 0;
 	struct dentry *privroot = REISERFS_SB(s)->priv_root;
 
-#ifdef CONFIG_REISERFS_FS_XATTR
 	err = xattr_mount_check(s);
 	if (err)
 		goto error;
@@ -1023,14 +1025,11 @@ error:
 		clear_bit(REISERFS_XATTRS_USER, &(REISERFS_SB(s)->s_mount_opt));
 		clear_bit(REISERFS_POSIXACL, &(REISERFS_SB(s)->s_mount_opt));
 	}
-#endif
 
 	/* The super_block MS_POSIXACL must mirror the (no)acl mount option. */
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
 	if (reiserfs_posixacl(s))
 		s->s_flags |= MS_POSIXACL;
 	else
-#endif
 		s->s_flags &= ~MS_POSIXACL;
 
 	return err;

[patch 2/4] [PATCH] reiserfs: deal with NULL xattr root w/ xattrs disabled

[Jeff Mahoney]

[-- Attachment #1: patches.fixes/reiserfs-xattr-root-fixup --]
[-- Type: text/plain, Size: 1302 bytes --]

 This patch avoids an Oops in open_xa_root that can occur when deleting a
 file with xattrs disabled. It assumes that the xattr root will be there,
 and that is not guaranteed.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/reiserfs/super.c |    2 --
 fs/reiserfs/xattr.c |    4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -448,13 +448,11 @@ int remove_save_link(struct inode *inode
 static void reiserfs_kill_sb(struct super_block *s)
 {
 	if (REISERFS_SB(s)) {
-#ifdef CONFIG_REISERFS_FS_XATTR
 		if (REISERFS_SB(s)->xattr_root) {
 			d_invalidate(REISERFS_SB(s)->xattr_root);
 			dput(REISERFS_SB(s)->xattr_root);
 			REISERFS_SB(s)->xattr_root = NULL;
 		}
-#endif
 		if (REISERFS_SB(s)->priv_root) {
 			d_invalidate(REISERFS_SB(s)->priv_root);
 			dput(REISERFS_SB(s)->priv_root);
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -123,7 +123,9 @@ static struct dentry *open_xa_root(struc
 	mutex_lock_nested(&privroot->d_inode->i_mutex, I_MUTEX_XATTR);
 
 	xaroot = dget(REISERFS_SB(sb)->xattr_root);
-	if (!xaroot->d_inode) {
+	if (!xaroot)
+		xaroot = ERR_PTR(-ENODATA);
+	else if (!xaroot->d_inode) {
 		int err = -ENODATA;
 		if (xattr_may_create(flags))
 			err = xattr_mkdir(privroot->d_inode, xaroot, 0700);

[patch 3/4] reiserfs: fixup perms when xattrs are disabled

[Jeff Mahoney]

[-- Attachment #1: patches.fixes/reiserfs-xattrs-disabled-perms --]
[-- Type: text/plain, Size: 2958 bytes --]

 This patch CONFIG_REISERFS_FS_XATTR protection from reiserfs_permission.

 This is needed to avoid warnings during file deletions and chowns with
 xattrs disabled.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/reiserfs/xattr.c            |   37 ++++++++++++++++++++-----------------
 include/linux/reiserfs_xattr.h |    4 +---
 2 files changed, 21 insertions(+), 20 deletions(-)

--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -869,23 +869,6 @@ static int reiserfs_check_acl(struct ino
 	return error;
 }
 
-int reiserfs_permission(struct inode *inode, int mask)
-{
-	/*
-	 * We don't do permission checks on the internal objects.
-	 * Permissions are determined by the "owning" object.
-	 */
-	if (IS_PRIVATE(inode))
-		return 0;
-	/*
-	 * Stat data v1 doesn't support ACLs.
-	 */
-	if (get_inode_sd_version(inode) == STAT_DATA_V1)
-		return generic_permission(inode, mask, NULL);
-	else
-		return generic_permission(inode, mask, reiserfs_check_acl);
-}
-
 static int create_privroot(struct dentry *dentry)
 {
 	int err;
@@ -949,6 +932,26 @@ static int xattr_mount_check(struct supe
 	return 0;
 }
 
+int reiserfs_permission(struct inode *inode, int mask)
+{
+	/*
+	 * We don't do permission checks on the internal objects.
+	 * Permissions are determined by the "owning" object.
+	 */
+	if (IS_PRIVATE(inode))
+		return 0;
+
+#ifdef CONFIG_REISERFS_FS_XATTR
+	/*
+	 * Stat data v1 doesn't support ACLs.
+	 */
+	if (get_inode_sd_version(inode) != STAT_DATA_V1)
+		return generic_permission(inode, mask, reiserfs_check_acl);
+	else
+#endif
+		return generic_permission(inode, mask, NULL);
+}
+
 /* This will catch lookups from the fs root to .reiserfs_priv */
 static int
 xattr_lookup_poison(struct dentry *dentry, struct qstr *q1, struct qstr *name)
--- a/include/linux/reiserfs_xattr.h
+++ b/include/linux/reiserfs_xattr.h
@@ -41,6 +41,7 @@ int reiserfs_xattr_init(struct super_blo
 int reiserfs_lookup_privroot(struct super_block *sb);
 int reiserfs_delete_xattrs(struct inode *inode);
 int reiserfs_chown_xattrs(struct inode *inode, struct iattr *attrs);
+int reiserfs_permission(struct inode *inode, int mask);
 
 #ifdef CONFIG_REISERFS_FS_XATTR
 #define has_xattr_dir(inode) (REISERFS_I(inode)->i_flags & i_has_xattr_dir)
@@ -50,7 +51,6 @@ int reiserfs_setxattr(struct dentry *den
 		      const void *value, size_t size, int flags);
 ssize_t reiserfs_listxattr(struct dentry *dentry, char *buffer, size_t size);
 int reiserfs_removexattr(struct dentry *dentry, const char *name);
-int reiserfs_permission(struct inode *inode, int mask);
 
 int reiserfs_xattr_get(struct inode *, const char *, void *, size_t);
 int reiserfs_xattr_set(struct inode *, const char *, const void *, size_t, int);
@@ -117,8 +117,6 @@ static inline void reiserfs_init_xattr_r
 #define reiserfs_listxattr NULL
 #define reiserfs_removexattr NULL
 
-#define reiserfs_permission NULL
-
 static inline void reiserfs_init_xattr_rwsem(struct inode *inode)
 {
 }

Re: [patch 3/4] reiserfs: fixup perms when xattrs are disabled

[Stefan Richter]

Jeff Mahoney wrote:
>  This patch CONFIG_REISERFS_FS_XATTR protection from reiserfs_permission.
> 
>  This is needed to avoid warnings during file deletions and chowns with
>  xattrs disabled.
> 
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Al has a minimally edited version in vfs-2.6.git/for-next:

[...]
> +#ifdef CONFIG_REISERFS_FS_XATTR
> +	/*
> +	 * Stat data v1 doesn't support ACLs.
> +	 */
> +	if (get_inode_sd_version(inode) != STAT_DATA_V1)
> +		return generic_permission(inode, mask, reiserfs_check_acl);
> +	else
> +#endif
> +		return generic_permission(inode, mask, NULL);
> +}
> +

The else is omitted, and hence the second return unindented.

Al, did you mean to let Linus pull from your tree once more?
(I have been using a reiserfs root FS with Jeff's four patches for a 
week and all went smoothly.)
-- 
Stefan Richter
-=====-=-=== -=-= -==-=
http://arcgraph.de/sr/

Re: [patch 3/4] reiserfs: fixup perms when xattrs are disabled

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stefan Richter wrote:
> Jeff Mahoney wrote:
>>  This patch CONFIG_REISERFS_FS_XATTR protection from reiserfs_permission.
>>
>>  This is needed to avoid warnings during file deletions and chowns with
>>  xattrs disabled.
>>
>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> 
> Al has a minimally edited version in vfs-2.6.git/for-next:
> 
> [...]
>> +#ifdef CONFIG_REISERFS_FS_XATTR
>> +    /*
>> +     * Stat data v1 doesn't support ACLs.
>> +     */
>> +    if (get_inode_sd_version(inode) != STAT_DATA_V1)
>> +        return generic_permission(inode, mask, reiserfs_check_acl);
>> +    else
>> +#endif
>> +        return generic_permission(inode, mask, NULL);
>> +}
>> +
> 
> The else is omitted, and hence the second return unindented.

Oops, of course. I'd forgotten about that once I thought Al had merged them.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iEYEARECAAYFAkoQGDgACgkQLPWxlyuTD7LQtwCcCPb3UC7sQann3C5j7ZvhGNIK
X2QAniRCmLRUazgPunHzzkXVK0XY04QZ
=c79O
-----END PGP SIGNATURE-----

[patch 4/4] reiserfs: allow exposing privroot w/ xattrs enabled

[Jeff Mahoney]

[-- Attachment #1: patches.fixes/reiserfs-expose-privroot --]
[-- Type: text/plain, Size: 2580 bytes --]

--- a/fs/reiserfs/dir.c
+++ b/fs/reiserfs/dir.c
@@ -44,13 +44,11 @@ static int reiserfs_dir_fsync(struct fil
 static inline bool is_privroot_deh(struct dentry *dir,
 				   struct reiserfs_de_head *deh)
 {
-	int ret = 0;
-#ifdef CONFIG_REISERFS_FS_XATTR
 	struct dentry *privroot = REISERFS_SB(dir->d_sb)->priv_root;
-	ret = (dir == dir->d_parent && privroot->d_inode &&
-	       deh->deh_objectid == INODE_PKEY(privroot->d_inode)->k_objectid);
-#endif
-	return ret;
+	if (reiserfs_expose_privroot(dir->d_sb))
+		return 0;
+	return (dir == dir->d_parent && privroot->d_inode &&
+	        deh->deh_objectid == INODE_PKEY(privroot->d_inode)->k_objectid);
 }
 
 int reiserfs_readdir_dentry(struct dentry *dentry, void *dirent,
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -898,6 +898,7 @@ static int reiserfs_parse_options(struct
 		{"conv",.setmask = 1 << REISERFS_CONVERT},
 		{"attrs",.setmask = 1 << REISERFS_ATTRS},
 		{"noattrs",.clrmask = 1 << REISERFS_ATTRS},
+		{"expose_privroot", .setmask = 1 << REISERFS_EXPOSE_PRIVROOT},
 #ifdef CONFIG_REISERFS_FS_XATTR
 		{"user_xattr",.setmask = 1 << REISERFS_XATTRS_USER},
 		{"nouser_xattr",.clrmask = 1 << REISERFS_XATTRS_USER},
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -982,7 +982,8 @@ int reiserfs_lookup_privroot(struct supe
 				strlen(PRIVROOT_NAME));
 	if (!IS_ERR(dentry)) {
 		REISERFS_SB(s)->priv_root = dentry;
-		s->s_root->d_op = &xattr_lookup_poison_ops;
+		if (!reiserfs_expose_privroot(s))
+			s->s_root->d_op = &xattr_lookup_poison_ops;
 		if (dentry->d_inode)
 			dentry->d_inode->i_flags |= S_PRIVATE;
 	} else
--- a/include/linux/reiserfs_fs_sb.h
+++ b/include/linux/reiserfs_fs_sb.h
@@ -453,6 +453,7 @@ enum reiserfs_mount_options {
 	REISERFS_ATTRS,
 	REISERFS_XATTRS_USER,
 	REISERFS_POSIXACL,
+	REISERFS_EXPOSE_PRIVROOT,
 	REISERFS_BARRIER_NONE,
 	REISERFS_BARRIER_FLUSH,
 
@@ -490,6 +491,7 @@ enum reiserfs_mount_options {
 #define reiserfs_data_writeback(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_DATA_WRITEBACK))
 #define reiserfs_xattrs_user(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_XATTRS_USER))
 #define reiserfs_posixacl(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_POSIXACL))
+#define reiserfs_expose_privroot(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_EXPOSE_PRIVROOT))
 #define reiserfs_xattrs_optional(s) (reiserfs_xattrs_user(s) || reiserfs_posixacl(s))
 #define reiserfs_barrier_none(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_BARRIER_NONE))
 #define reiserfs_barrier_flush(s) (REISERFS_SB(s)->s_mount_opt & (1 << REISERFS_BARRIER_FLUSH))

Re: [patch 4/4] reiserfs: allow exposing privroot w/ xattrs enabled

[Linus Torvalds]

On Sun, 17 May 2009, Jeff Mahoney wrote:
>
> --- a/fs/reiserfs/dir.c
> +++ b/fs/reiserfs/dir.c

Hmm. This had no explanation or sign-off. Please? At least the sign-off, 
but I think I'd like a log message too, since this adds a new mount option 
etc, so it's not like this is some trivial one-liner that doesn't need it.

		Linus

Re: [patch 4/4] reiserfs: allow exposing privroot w/ xattrs enabled

[Linus Torvalds]

On Sun, 17 May 2009, Linus Torvalds wrote:
> 
> 
> On Sun, 17 May 2009, Jeff Mahoney wrote:
> >
> > --- a/fs/reiserfs/dir.c
> > +++ b/fs/reiserfs/dir.c
> 
> Hmm. This had no explanation or sign-off. Please? At least the sign-off, 
> but I think I'd like a log message too, since this adds a new mount option 
> etc, so it's not like this is some trivial one-liner that doesn't need it.

Jeff, ping?

I merged the other three patches, but this one is still pending. I'm not 
entirely sure it's even appropriate for 2.6.30 (it doesn't _look_ like 
it's fixing any regressions), but even if you don't actually want me to 
merge it, can you say so? And if you do want me to merge it, I want the 
sign-off and fuller log message..

		Linus

Re: [patch 4/4] reiserfs: allow exposing privroot w/ xattrs enabled

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Linus Torvalds wrote:
> 
> On Sun, 17 May 2009, Linus Torvalds wrote:
>>
>> On Sun, 17 May 2009, Jeff Mahoney wrote:
>>> --- a/fs/reiserfs/dir.c
>>> +++ b/fs/reiserfs/dir.c
>> Hmm. This had no explanation or sign-off. Please? At least the sign-off, 
>> but I think I'd like a log message too, since this adds a new mount option 
>> etc, so it's not like this is some trivial one-liner that doesn't need it.
> 
> Jeff, ping?
> 
> I merged the other three patches, but this one is still pending. I'm not 
> entirely sure it's even appropriate for 2.6.30 (it doesn't _look_ like 
> it's fixing any regressions), but even if you don't actually want me to 
> merge it, can you say so? And if you do want me to merge it, I want the 
> sign-off and fuller log message..

Sorry, I was in and out all weekend. Don't bother merging it for this
window. I wanted to make it easy to clean up xattrs that would have been
left around if a file system that contained xattrs was migrated to a
kernel without support compiled in, but I'll add that to fsck instead.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iEYEARECAAYFAkoRnHUACgkQLPWxlyuTD7IXNwCfXj5l+0m0RTPQqzdD2VUqJep9
aEkAoIVdu7anayn2pP5v1dJAv18s6/4i
=HKsW
-----END PGP SIGNATURE-----

Re: [patch 4/4] reiserfs: allow exposing privroot w/ xattrs enabled

[Linus Torvalds]

On Mon, 18 May 2009, Jeff Mahoney wrote:
> 
> Sorry, I was in and out all weekend. Don't bother merging it for this
> window. I wanted to make it easy to clean up xattrs that would have been
> left around if a file system that contained xattrs was migrated to a
> kernel without support compiled in, but I'll add that to fsck instead.

Ok, thanks, patch happily forgotten,

		Linus

[PATCH 0/4] reiserfs: fixes

[Christian Brauner]

Hey Christoph & Jan,

A series of smaller fixes for reiserfs including one deadlock reported
by syzbot (albeit with a bogus bisection). Plan would be to get this
merged within -rc6.

I've actually tested all this with xfstests. With and without this
patch series the same 18 tests fail.

Thanks!
Christian


---
base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3
change-id: 20231009-vfs-fixes-reiserfs-3402fe7abb94