ext4: don't treat fhandle lookup of ea_inode as FS corruption

A file handle that userspace provides to open_by_handle_at() can
legitimately contain an outdated inode number that has since been reused
for another purpose - that's why the file handle also contains a generation
number.

But if the inode number has been reused for an ea_inode, check_igot_inode()
will notice, __ext4_iget() will go through ext4_error_inode(), and if the
inode was newly created, it will also be marked as bad by iget_failed().
This all happens before the point where the inode generation is checked.

ext4_error_inode() is supposed to only be used on filesystem corruption; it
should not be used when userspace just got unlucky with a stale file
handle. So when this happens, let __ext4_iget() just return an error.

Fixes: b3e6bcb94590 ("ext4: add EA_INODE checking to ext4_iget()")
Signed-off-by: Jann Horn <jannh@google.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20241129-ext4-ignore-ea-fhandle-v1-1-e532c0d1cee0@google.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index aede80fa17814318ee7b4c9439f39adc560e25cb..5c57a34cd82c9290aa7f2cb1aecd1e13052a6625 100644 (file)
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4732,22 +4732,43 @@ static inline void ext4_inode_set_iversion_queried(struct inode *inode, u64 val)
                inode_set_iversion_queried(inode, val);
 }
 
-static const char *check_igot_inode(struct inode *inode, ext4_iget_flags flags)
-
+static int check_igot_inode(struct inode *inode, ext4_iget_flags flags,
+                           const char *function, unsigned int line)
 {
+       const char *err_str;
+
        if (flags & EXT4_IGET_EA_INODE) {
-               if (!(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL))
-                       return "missing EA_INODE flag";
+               if (!(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)) {
+                       err_str = "missing EA_INODE flag";
+                       goto error;
+               }
                if (ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
-                   EXT4_I(inode)->i_file_acl)
-                       return "ea_inode with extended attributes";
+                   EXT4_I(inode)->i_file_acl) {
+                       err_str = "ea_inode with extended attributes";
+                       goto error;
+               }
        } else {
-               if ((EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL))
-                       return "unexpected EA_INODE flag";
+               if ((EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)) {
+                       /*
+                        * open_by_handle_at() could provide an old inode number
+                        * that has since been reused for an ea_inode; this does
+                        * not indicate filesystem corruption
+                        */
+                       if (flags & EXT4_IGET_HANDLE)
+                               return -ESTALE;
+                       err_str = "unexpected EA_INODE flag";
+                       goto error;
+               }
+       }
+       if (is_bad_inode(inode) && !(flags & EXT4_IGET_BAD)) {
+               err_str = "unexpected bad inode w/o EXT4_IGET_BAD";
+               goto error;
        }
-       if (is_bad_inode(inode) && !(flags & EXT4_IGET_BAD))
-               return "unexpected bad inode w/o EXT4_IGET_BAD";
-       return NULL;
+       return 0;
+
+error:
+       ext4_error_inode(inode, function, line, 0, err_str);
+       return -EFSCORRUPTED;
 }
 
 struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
@@ -4759,7 +4780,6 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
        struct ext4_inode_info *ei;
        struct ext4_super_block *es = EXT4_SB(sb)->s_es;
        struct inode *inode;
-       const char *err_str;
        journal_t *journal = EXT4_SB(sb)->s_journal;
        long ret;
        loff_t size;
@@ -4788,10 +4808,10 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
        if (!inode)
                return ERR_PTR(-ENOMEM);
        if (!(inode->i_state & I_NEW)) {
-               if ((err_str = check_igot_inode(inode, flags)) != NULL) {
-                       ext4_error_inode(inode, function, line, 0, err_str);
+               ret = check_igot_inode(inode, flags, function, line);
+               if (ret) {
                        iput(inode);
-                       return ERR_PTR(-EFSCORRUPTED);
+                       return ERR_PTR(ret);
                }
                return inode;
        }
@@ -5073,13 +5093,21 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
                ret = -EFSCORRUPTED;
                goto bad_inode;
        }
-       if ((err_str = check_igot_inode(inode, flags)) != NULL) {
-               ext4_error_inode(inode, function, line, 0, err_str);
-               ret = -EFSCORRUPTED;
-               goto bad_inode;
+       ret = check_igot_inode(inode, flags, function, line);
+       /*
+        * -ESTALE here means there is nothing inherently wrong with the inode,
+        * it's just not an inode we can return for an fhandle lookup.
+        */
+       if (ret == -ESTALE) {
+               brelse(iloc.bh);
+               unlock_new_inode(inode);
+               iput(inode);
+               return ERR_PTR(-ESTALE);
        }
-
+       if (ret)
+               goto bad_inode;
        brelse(iloc.bh);
+
        unlock_new_inode(inode);
        return inode;