lkml.org 
[lkml]   [2018]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
Subject[PATCH] reiserfs: propagate errors from fill_with_dentries properly
From
fill_with_dentries() failed to propagate errors up to
reiserfs_for_each_xattr() properly. Plumb them through.

Note that reiserfs_for_each_xattr() is only used by
reiserfs_delete_xattrs() and reiserfs_chown_xattrs().
The result of reiserfs_delete_xattrs() is discarded anyway, the only
difference there is whether a warning is printed to dmesg.
The result of reiserfs_chown_xattrs() does matter because it can block
chowning of the file to which the xattrs belong; but either way, the
resulting state can have misaligned ownership, so my patch doesn't improve
things greatly.

Credit for making me look at this code goes to Al Viro, who pointed
out that the ->actor calling convention is suboptimal and should be
changed.

Signed-off-by: Jann Horn <jannh@google.com>
---
I have tested this by manually patching error injection into
fill_with_dentries().

Opinions? Is this a sensible change?

Because the changes in this patch are more superficial than the changes
in the other one, I split this out so that the security patch is a
clean change that obviously belongs in stable and can hopefully go in
quickly.

After the cases I can see where errors are returned improperly are
cleaned up, I plan to change the calling convention for ->actor as
suggested by Al Viro.

fs/reiserfs/xattr.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index ff94fad477e4..ae4a28410dbd 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -185,6 +185,7 @@ struct reiserfs_dentry_buf {
struct dir_context ctx;
struct dentry *xadir;
int count;
+ int err;
struct dentry *dentries[8];
};

@@ -207,6 +208,7 @@ fill_with_dentries(struct dir_context *ctx, const char *name, int namelen,

dentry = lookup_one_len(name, dbuf->xadir, namelen);
if (IS_ERR(dentry)) {
+ dbuf->err = PTR_ERR(dentry);
return PTR_ERR(dentry);
} else if (d_really_is_negative(dentry)) {
/* A directory entry exists, but no file? */
@@ -215,6 +217,7 @@ fill_with_dentries(struct dir_context *ctx, const char *name, int namelen,
"not found for file %pd.\n",
dentry, dbuf->xadir);
dput(dentry);
+ dbuf->err = -EIO;
return -EIO;
}

@@ -262,6 +265,10 @@ static int reiserfs_for_each_xattr(struct inode *inode,
err = reiserfs_readdir_inode(d_inode(dir), &buf.ctx);
if (err)
break;
+ if (buf.err) {
+ err = buf.err;
+ break;
+ }
if (!buf.count)
break;
for (i = 0; !err && i < buf.count && buf.dentries[i]; i++) {
--
2.18.0.597.ga71716f1ad-goog
\
 
 \ /
  Last update: 2018-08-02 18:34    [W:0.036 / U:1.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site