ext4: avoid declaring fs inconsistent due to invalid file handles

PATCH In review Theodore Ts'o 2 participants 6 messages Dec 13, 2018 – Dec 19, 2018 ext4 file system

Share & tools

Theodore Ts'o

author maintainer

2018-12-13 17:51 UTC #0 lore raw

PATCH 1/1 ext4: avoid declaring fs inconsistent due to invalid file handles

If we receive a file handle, either from NFS or open_by_handle_at(2), and it points at an inode which has not been initialized, and the file system has metadata checksums enabled, we shouldn't try to get the inode, discover the checksum is invalid, and then declare the file system as being inconsistent.

This can be reproduced by creating a test file system via "mke2fs -t ext4 -O metadata_csum /tmp/foo.img 8M", mounting it, cd'ing into that directory, and then running the following program.

#define _GNU_SOURCE
#include <fcntl.h>

struct handle {
	struct file_handle fh;
	unsigned char fid[MAX_HANDLE_SZ];
};

int main(int argc, char **argv)

{
	struct handle h = {{8, 1 }, { 12, }};

	open_by_handle_at(AT_FDCWD, &h.fh, O_RDONLY);
	return 0;
}

Addresses-Google-Bug: #120690101

Signed-off-by Theodore Ts'o <[email protected]>

Cc [email protected]

---
 fs/ext4/ext4.h   | 12 +++++++++--
 fs/ext4/ialloc.c |  2 +-
 fs/ext4/inode.c  | 54 +++++++++++++++++++++++++++++++++---------------
 fs/ext4/ioctl.c  |  2 +-
 fs/ext4/namei.c  |  4 ++--
 fs/ext4/resize.c |  4 ++--
 fs/ext4/super.c  | 19 +++++------------
 fs/ext4/xattr.c  |  5 +++--
 8 files changed, 61 insertions(+), 41 deletions(-)

fs/ext4/ext4.h +10 -2

index b4621277e259..20eaa66705cc 100644
@@ -2454,8 +2454,16 @@ int do_journal_get_write_access(handle_t *handle,
 #define FALL_BACK_TO_NONDELALLOC 1
 #define CONVERT_INLINE_DATA	 2
 
-extern struct inode *ext4_iget(struct super_block *, unsigned long);
-extern struct inode *ext4_iget_normal(struct super_block *, unsigned long);
+extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
+				 int flags, const char *function,
+				 unsigned int line);
+
+#define EXT4_IGET_NORMAL	1
+#define EXT4_IGET_HANDLE	2
+
+#define ext4_iget(sb, ino, flags) \
+	__ext4_iget((sb), (ino), (flags), __func__, __LINE__)
+
 extern int  ext4_write_inode(struct inode *, struct writeback_control *);
 extern int  ext4_setattr(struct dentry *, struct iattr *);
 extern int  ext4_getattr(const struct path *, struct kstat *, u32, unsigned int);

fs/ext4/ialloc.c +1 -1

index 014f6a698cb7..06aa3b6c3f18 100644
@@ -1225,7 +1225,7 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
 	if (!ext4_test_bit(bit, bitmap_bh->b_data))
 		goto bad_orphan;
 
-	inode = ext4_iget(sb, ino);
+	inode = ext4_iget(sb, ino, 0);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		ext4_error(sb, "couldn't read orphan inode %lu (err %d)",

fs/ext4/inode.c +37 ~~-17~~

117 lines not on this page · raw message

fs/ext4/ioctl.c +1 -1

index 0edee31913d1..03a45f669b2e 100644
@@ -125,7 +125,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
 	    !inode_owner_or_capable(inode) || !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO);
+	inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO, 0);
 	if (IS_ERR(inode_bl))
 		return PTR_ERR(inode_bl);
 	ei_bl = EXT4_I(inode_bl);

fs/ext4/namei.c +2 -2

index 437f71fe83ae..2b928eb07fa2 100644
@@ -1571,7 +1571,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi
 					 dentry);
 			return ERR_PTR(-EFSCORRUPTED);
 		}
-		inode = ext4_iget_normal(dir->i_sb, ino);
+		inode = ext4_iget(dir->i_sb, ino, EXT4_IGET_NORMAL);
 		if (inode == ERR_PTR(-ESTALE)) {
 			EXT4_ERROR_INODE(dir,
 					 "deleted inode referenced: %u",

@@ -1613,7 +1613,7 @@ struct dentry *ext4_get_parent(struct dentry *child)
 		return ERR_PTR(-EFSCORRUPTED);
 	}
 
-	return d_obtain_alias(ext4_iget_normal(child->d_sb, ino));
+	return d_obtain_alias(ext4_iget(child->d_sb, ino, EXT4_IGET_NORMAL));
 }
 
 /*

fs/ext4/resize.c +2 -2

index bc8ee0c498cc..283f8c752215 100644
@@ -1637,7 +1637,7 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
 				     "No reserved GDT blocks, can't resize");
 			return -EPERM;
 		}
-		inode = ext4_iget(sb, EXT4_RESIZE_INO);
+		inode = ext4_iget(sb, EXT4_RESIZE_INO, 0);
 		if (IS_ERR(inode)) {
 			ext4_warning(sb, "Error opening resize inode");
 			return PTR_ERR(inode);

@@ -1965,7 +1965,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 		}
 
 		if (!resize_inode)
-			resize_inode = ext4_iget(sb, EXT4_RESIZE_INO);
+			resize_inode = ext4_iget(sb, EXT4_RESIZE_INO, 0);
 		if (IS_ERR(resize_inode)) {
 			ext4_warning(sb, "Error opening resize inode");
 			return PTR_ERR(resize_inode);

fs/ext4/super.c +5 ~~-14~~

index e862b82066ab..aa45c233c4cf 100644
@@ -1173,20 +1173,11 @@ static struct inode *ext4_nfs_get_inode(struct super_block *sb,
 {
 	struct inode *inode;
 
-	if (ino < EXT4_FIRST_INO(sb) && ino != EXT4_ROOT_INO)
-		return ERR_PTR(-ESTALE);
-	if (ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))
-		return ERR_PTR(-ESTALE);
-
-	/* iget isn't really right if the inode is currently unallocated!!
-	 *
-	 * ext4_read_inode will return a bad_inode if the inode had been
-	 * deleted, so we should be safe.
-	 *
+	/*
 	 * Currently we don't know the generation for parent directory, so
 	 * a generation of 0 means "accept any"
 	 */
-	inode = ext4_iget_normal(sb, ino);
+	inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL | EXT4_IGET_HANDLE);
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 	if (generation && inode->i_generation != generation) {

@@ -4350,7 +4341,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * so we can safely mount the rest of the filesystem now.
 	 */
 
-	root = ext4_iget(sb, EXT4_ROOT_INO);
+	root = ext4_iget(sb, EXT4_ROOT_INO, 0);
 	if (IS_ERR(root)) {
 		ext4_msg(sb, KERN_ERR, "get root inode failed");
 		ret = PTR_ERR(root);

@@ -4618,7 +4609,7 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 	 * happen if we iget() an unused inode, as the subsequent iput()
 	 * will try to delete it.
 	 */
-	journal_inode = ext4_iget(sb, journal_inum);
+	journal_inode = ext4_iget(sb, journal_inum, 0);
 	if (IS_ERR(journal_inode)) {
 		ext4_msg(sb, KERN_ERR, "no journal found");
 		return NULL;

@@ -5700,7 +5691,7 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
 	if (!qf_inums[type])
 		return -EPERM;
 
-	qf_inode = ext4_iget(sb, qf_inums[type]);
+	qf_inode = ext4_iget(sb, qf_inums[type], 0);
 	if (IS_ERR(qf_inode)) {
 		ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
 		return PTR_ERR(qf_inode);

fs/ext4/xattr.c +3 -2

index 510c9bb7ce71..7fddd519adae 100644
@@ -384,7 +384,7 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
 	struct inode *inode;
 	int err;
 
-	inode = ext4_iget(parent->i_sb, ea_ino);
+	inode = ext4_iget(parent->i_sb, ea_ino, EXT4_IGET_NORMAL);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		ext4_error(parent->i_sb,

@@ -1482,7 +1482,8 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 	}
 
 	while (ce) {
-		ea_inode = ext4_iget(inode->i_sb, ce->e_value);
+		ea_inode = ext4_iget(inode->i_sb, ce->e_value,
+				     EXT4_IGET_NORMAL);
 		if (!IS_ERR(ea_inode) &&
 		    !is_bad_inode(ea_inode) &&
 		    (EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL) &&

Signature · 1 line

2.19.1

Andreas Dilger

2018-12-17 22:54 UTC #1 lore raw

Quoted 49 lines · Theodore Ts'o

If we receive a file handle, either from NFS or open_by_handle_at(2), and it points at an inode which has not been initialized, and the file system has metadata checksums enabled, we shouldn't try to get the inode, discover the checksum is invalid, and then declare the file system as being inconsistent.
This can be reproduced by creating a test file system via "mke2fs -t ext4 -O metadata_csum /tmp/foo.img 8M", mounting it, cd'ing into that directory, and then running the following program.
#define _GNU_SOURCE
#include <fcntl.h>
struct handle {
	struct file_handle fh;
	unsigned char fid[MAX_HANDLE_SZ];
};
int main(int argc, char **argv)
{
	struct handle h = {{8, 1 }, { 12, }};

	open_by_handle_at(AT_FDCWD, &h.fh, O_RDONLY);
	return 0;
}
Addresses-Google-Bug: #120690101
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
---
fs/ext4/ext4.h   | 12 +++++++++--
fs/ext4/ialloc.c |  2 +-
fs/ext4/inode.c  | 54 +++++++++++++++++++++++++++++++++---------------
fs/ext4/ioctl.c  |  2 +-
fs/ext4/namei.c  |  4 ++--
fs/ext4/resize.c |  4 ++--
fs/ext4/super.c  | 19 +++++------------
fs/ext4/xattr.c  |  5 +++--
8 files changed, 61 insertions(+), 41 deletions(-)
fs/ext4/ext4.h +0 -0
index b4621277e259..20eaa66705cc 100644
@@ -2454,8 +2454,16 @@ int do_journal_get_write_access(handle_t *handle,
 #define FALL_BACK_TO_NONDELALLOC 1

fs/ext4/ext4.h int do_journal_get_write_access(handle_t *handle, · line 2462

Quoted 10 lines

fs/ext4/ext4.h +6 -2

 #define CONVERT_INLINE_DATA	 2
 
-extern struct inode *ext4_iget(struct super_block *, unsigned long);
-extern struct inode *ext4_iget_normal(struct super_block *, unsigned long);
+extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
+				 int flags, const char *function,
+				 unsigned int line);
+
+#define EXT4_IGET_NORMAL	1
+#define EXT4_IGET_HANDLE	2

It would be better to make this:

 enum ext4_iget_flags {
        EXT4_IGET_RESERVED = 0x00,	/* just guessing, see further below */
	EXT4_IGET_NORMAL   = 0x01,
	EXT4_IGET_HANDLE   = 0x02,

};

so that it is clear which "flags" are being passed to ext4_iget(). There are hundreds of "flags" arguments to different functions that are sometimes hard to separate when looking at code. That also gives the compiler some chance to warn if they are used incorrectly.

Quoted 11 lines

fs/ext4/ext4.h +3 -0

+#define ext4_iget(sb, ino, flags) \
+	__ext4_iget((sb), (ino), (flags), __func__, __LINE__)
+
 extern int  ext4_write_inode(struct inode *, struct writeback_control *);
 extern int  ext4_setattr(struct dentry *, struct iattr *);
 extern int  ext4_getattr(const struct path *, struct kstat *, u32, unsigned int);

fs/ext4/ialloc.c +0 -0

index 014f6a698cb7..06aa3b6c3f18 100644

fs/ext4/ialloc.c struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino) · line 1228

Quoted 5 lines

fs/ext4/ialloc.c +1 -1

@@ -1225,7 +1225,7 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
 	if (!ext4_test_bit(bit, bitmap_bh->b_data))
 		goto bad_orphan;
 
-	inode = ext4_iget(sb, ino);
+	inode = ext4_iget(sb, ino, 0);

What does "0" mean? It isn't in the list of EXT4_IGET_* flags. I'm guessing it means that access to reserved or otherwise invalid inodes is allowed?

fs/ext4/inode.c static inline u64 ext4_inode_peek_iversion(const struct inode *inode) · line 4821

Quoted 11 lines

fs/ext4/inode.c +2 -1

index 22a9d8159720..07fc7a12508e 100644
@@ -4817,7 +4817,9 @@ static inline u64 ext4_inode_peek_iversion(const struct inode *inode)
 		return inode_peek_iversion(inode);
 }
 
-struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
+struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
+			  int flags, const char *function,

enum ext4_iget_flags flags, ...

fs/ext4/inode.c struct inode *ext4_iget(struct super_block *sb, unsigned long ino) · line 4838

Quoted 7 lines

fs/ext4/inode.c +3 -0

@@ -4831,6 +4833,18 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 	gid_t i_gid;
 	projid_t i_projid;
 
+	if (((flags & EXT4_IGET_NORMAL) &&
+	     (ino < EXT4_FIRST_INO(sb) && ino != EXT4_ROOT_INO)) ||
+	    (ino < EXT4_ROOT_INO) ||

This check seems redundant with "ino < EXT4_FIRST_INO(sb)" on the previous line?

fs/ext4/inode.c struct inode *ext4_iget(struct super_block *sb, unsigned long ino) · line 4846

Quoted 8 lines

fs/ext4/inode.c +8 -0

+	    (ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))) {
+		if (flags & EXT4_IGET_HANDLE)
+			return ERR_PTR(-ESTALE);
+		__ext4_error(sb, function, line,
+			     "inode #%lu: comm %s: iget: illegal inode #",
+			     ino, current->comm);
+		return ERR_PTR(-EFSCORRUPTED);
+	}

Cheers, Andreas

Theodore Y. Ts'o

author maintainer

2018-12-18 04:45 UTC #2 lore raw

Quoted 9 lines · Andreas Dilger

fs/ext4/ext4.h int do_journal_get_write_access(handle_t *handle, · line 2462
Quoted 2 lines
fs/ext4/ext4.h +2 -0
+#define EXT4_IGET_NORMAL	1
+#define EXT4_IGET_HANDLE	2
It would be better to make this:
 enum ext4_iget_flags {
        EXT4_IGET_RESERVED = 0x00,	/* just guessing, see further below */
	EXT4_IGET_NORMAL   = 0x01,
	EXT4_IGET_HANDLE   = 0x02,

Quoted 7 lines

};
fs/ext4/ialloc.c struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino) · line 1228
Quoted 2 lines
fs/ext4/ialloc.c +1 -1
-	inode = ext4_iget(sb, ino);
+	inode = ext4_iget(sb, ino, 0);
What does "0" mean? It isn't in the list of EXT4_IGET_* flags. I'm guessing it means that access to reserved or otherwise invalid inodes is allowed?

The flags are boolean OR'ed together, much like O_TRUNC | O_CREAT, etc. So an enum isn't really appropriate. So 0 means we're not enforcing "must be a normal inode" rules, and we're also not going to avoid throwing an EXT4_ERROR if the inode number is invalid.

I had thought it was obvious that flags can be or'ed together, and that "modes" are what might use an enum. I personally like flags because the can be more expressive, although I can see that "modes" are simpler since there is a much smaller set of valid modes, and you don't have to worry about define what happens when flags interact in unusual/unexpected ways.

It sounds like should add more explicit documentation at the very least so it's more clear what's going on.

- Ted

Andreas Dilger

2018-12-18 05:44 UTC #3 lore raw

Quoted 20 lines · Theodore Y. Ts'o

Quoted 11 lines · Andreas Dilger
fs/ext4/ext4.h int do_journal_get_write_access(handle_t *handle, · line 2462
Quoted 2 lines
fs/ext4/ext4.h +2 -0
+#define EXT4_IGET_NORMAL	1
+#define EXT4_IGET_HANDLE	2
It would be better to make this:
enum ext4_iget_flags {
       EXT4_IGET_RESERVED = 0x00,	/* just guessing, see further below */
	EXT4_IGET_NORMAL   = 0x01,
	EXT4_IGET_HANDLE   = 0x02,
};
Quoted 6 lines
fs/ext4/ialloc.c struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino) · line 1228
Quoted 2 lines
fs/ext4/ialloc.c +1 -1
-	inode = ext4_iget(sb, ino);
+	inode = ext4_iget(sb, ino, 0);
What does "0" mean? It isn't in the list of EXT4_IGET_* flags. I'm guessing it means that access to reserved or otherwise invalid inodes is allowed?

Quoted 2 lines

The flags are boolean OR'ed together, much like O_TRUNC | O_CREAT, etc. So an enum isn't really appropriate.

I don't think that it is verboten to use binary flag values in an enum, if you explicitly specify the values, which is why I used "0x01", "0x02" to make it more clear these are binary values. IMHO, using a named enum is a good way to annotate the function parameters rather than a generic "int flag" parameter that is ambiguous unless you look at the function code to see what the values of "flag" might be.

Quoted 3 lines

So 0 means we're not enforcing "must be a normal inode" rules, and we're also not going to avoid throwing an EXT4_ERROR if the inode number is invalid.

So that matches what I reverse engineered, which was EXT4_IGET_RESERVED but might have a better name if you can think of it. I originally had EXT4_IGET_NONE = 0, but I don't think that is quite correct.

Quoted 2 lines

I had thought it was obvious that flags can be or'ed together, and that "modes" are what might use an enum.

Their definition as "1" and "2" didn't make it clear that the next possible value was "4" and not "3".

Quoted 4 lines

I personally like flags because the can be more expressive, although I can see that "modes" are simpler since there is a much smaller set of valid modes, and you don't have to worry about define what happens when flags interact in unusual/unexpected ways.

I'm not against flags, and I figured out that they were orthogonal flags after reading the whole patch.

Quoted 4 lines

It sounds like should add more explicit documentation at the very least so it's more clear what's going on.
- Ted

Cheers, Andreas

Theodore Y. Ts'o

author maintainer

2018-12-18 16:43 UTC #4 lore raw

Quoted 6 lines · Andreas Dilger

I don't think that it is verboten to use binary flag values in an enum, if you explicitly specify the values, which is why I used "0x01", "0x02" to make it more clear these are binary values. IMHO, using a named enum is a good way to annotate the function parameters rather than a generic "int flag" parameter that is ambiguous unless you look at the function code to see what the values of "flag" might be.

I tend to only use enums in this kind of way:

enum classification_levels {
FOR_OFFICIAL_USE_ONLY,
CONFIDENTIAL,
SECRET,
TOP_SECRET,
};

I think the reason why I've never used it for type checking is because for gcc and sparse, it doesn't work. For the below example, "gcc -Wall foo.c" won't complain at all. Sparse complains only about the "return a | b;" line, because we're combining two different enum types. Sparse doesn't say boo that I passed EXT4_IGET_NORMAL where a classification_levels, and secret where an ext4_iget_flags is expected:

enum ext4_iget_flags {
	EXT4_IGET_RESERVED = 0x00,    /* just guessing, see further below */
	EXT4_IGET_NORMAL   = 0x01,
	EXT4_IGET_HANDLE   = 0x02
};

int combine(enum classification_levels a, enum ext4_iget_flags b)

{
	return a | b;
}

int main(int argc, char **argv)

{
	printf("%d\n", combine(EXT4_IGET_NORMAL, secret));
	exit(1);
}

Then again, llvm does correctly complain, and at least for Android configs, llvm will complain kernels correctly (although I'm not sure enterprise distros trust LLVM just yet), and I do agree that it's useful from a documentation perspective.

Cheers,

- Ted

Theodore Ts'o

author maintainer

2018-12-19 17:31 UTC #5 lore raw

PATCH 1/1 ext4: avoid declaring fs inconsistent due to invalid file handles

This can be reproduced by creating a test file system via "mke2fs -t ext4 -O metadata_csum /tmp/foo.img 8M", mounting it, cd'ing into that directory, and then running the following program.

#define _GNU_SOURCE
#include <fcntl.h>

struct handle {
	struct file_handle fh;
	unsigned char fid[MAX_HANDLE_SZ];
};

int main(int argc, char **argv)

{
	struct handle h = {{8, 1 }, { 12, }};

	open_by_handle_at(AT_FDCWD, &h.fh, O_RDONLY);
	return 0;
}

Google-Bug-Id: 120690101

Signed-off-by Theodore Ts'o <[email protected]>

Cc [email protected]

---
 fs/ext4/ext4.h   | 15 ++++++++++++--
 fs/ext4/ialloc.c |  2 +-
 fs/ext4/inode.c  | 54 +++++++++++++++++++++++++++++++++---------------
 fs/ext4/ioctl.c  |  2 +-
 fs/ext4/namei.c  |  4 ++--
 fs/ext4/resize.c |  5 +++--
 fs/ext4/super.c  | 19 +++++------------
 fs/ext4/xattr.c  |  5 +++--
 8 files changed, 65 insertions(+), 41 deletions(-)

fs/ext4/ext4.h +13 -2

index b4621277e259..185a05d3257e 100644
@@ -2454,8 +2454,19 @@ int do_journal_get_write_access(handle_t *handle,
 #define FALL_BACK_TO_NONDELALLOC 1
 #define CONVERT_INLINE_DATA	 2
 
-extern struct inode *ext4_iget(struct super_block *, unsigned long);
-extern struct inode *ext4_iget_normal(struct super_block *, unsigned long);
+typedef enum {
+	EXT4_IGET_NORMAL =	0,
+	EXT4_IGET_SPECIAL =	0x0001, /* OK to iget a system inode */
+	EXT4_IGET_HANDLE = 	0x0002	/* Inode # is from a handle */
+} ext4_iget_flags;
+
+extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
+				 ext4_iget_flags flags, const char *function,
+				 unsigned int line);
+
+#define ext4_iget(sb, ino, flags) \
+	__ext4_iget((sb), (ino), (flags), __func__, __LINE__)
+
 extern int  ext4_write_inode(struct inode *, struct writeback_control *);
 extern int  ext4_setattr(struct dentry *, struct iattr *);
 extern int  ext4_getattr(const struct path *, struct kstat *, u32, unsigned int);

fs/ext4/ialloc.c +1 -1

index 014f6a698cb7..7ff14a1adba3 100644
@@ -1225,7 +1225,7 @@ struct inode *ext4_orphan_get(struct super_block *sb, unsigned long ino)
 	if (!ext4_test_bit(bit, bitmap_bh->b_data))
 		goto bad_orphan;
 
-	inode = ext4_iget(sb, ino);
+	inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		ext4_error(sb, "couldn't read orphan inode %lu (err %d)",

fs/ext4/inode.c +37 ~~-17~~

117 lines not on this page · raw message

fs/ext4/ioctl.c +1 -1

index 0edee31913d1..d37dafa1d133 100644
@@ -125,7 +125,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
 	    !inode_owner_or_capable(inode) || !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO);
+	inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO, EXT4_IGET_SPECIAL);
 	if (IS_ERR(inode_bl))
 		return PTR_ERR(inode_bl);
 	ei_bl = EXT4_I(inode_bl);

fs/ext4/namei.c +2 -2

index 437f71fe83ae..2b928eb07fa2 100644
@@ -1571,7 +1571,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi
 					 dentry);
 			return ERR_PTR(-EFSCORRUPTED);
 		}
-		inode = ext4_iget_normal(dir->i_sb, ino);
+		inode = ext4_iget(dir->i_sb, ino, EXT4_IGET_NORMAL);
 		if (inode == ERR_PTR(-ESTALE)) {
 			EXT4_ERROR_INODE(dir,
 					 "deleted inode referenced: %u",

@@ -1613,7 +1613,7 @@ struct dentry *ext4_get_parent(struct dentry *child)
 		return ERR_PTR(-EFSCORRUPTED);
 	}
 
-	return d_obtain_alias(ext4_iget_normal(child->d_sb, ino));
+	return d_obtain_alias(ext4_iget(child->d_sb, ino, EXT4_IGET_NORMAL));
 }
 
 /*

fs/ext4/resize.c +3 -2

index bc8ee0c498cc..48421de803b7 100644
@@ -1637,7 +1637,7 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
 				     "No reserved GDT blocks, can't resize");
 			return -EPERM;
 		}
-		inode = ext4_iget(sb, EXT4_RESIZE_INO);
+		inode = ext4_iget(sb, EXT4_RESIZE_INO, EXT4_IGET_SPECIAL);
 		if (IS_ERR(inode)) {
 			ext4_warning(sb, "Error opening resize inode");
 			return PTR_ERR(inode);

@@ -1965,7 +1965,8 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
 		}
 
 		if (!resize_inode)
-			resize_inode = ext4_iget(sb, EXT4_RESIZE_INO);
+			resize_inode = ext4_iget(sb, EXT4_RESIZE_INO,
+						 EXT4_IGET_SPECIAL);
 		if (IS_ERR(resize_inode)) {
 			ext4_warning(sb, "Error opening resize inode");
 			return PTR_ERR(resize_inode);

fs/ext4/super.c +5 ~~-14~~

index e862b82066ab..fc9071081600 100644
@@ -1173,20 +1173,11 @@ static struct inode *ext4_nfs_get_inode(struct super_block *sb,
 {
 	struct inode *inode;
 
-	if (ino < EXT4_FIRST_INO(sb) && ino != EXT4_ROOT_INO)
-		return ERR_PTR(-ESTALE);
-	if (ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))
-		return ERR_PTR(-ESTALE);
-
-	/* iget isn't really right if the inode is currently unallocated!!
-	 *
-	 * ext4_read_inode will return a bad_inode if the inode had been
-	 * deleted, so we should be safe.
-	 *
+	/*
 	 * Currently we don't know the generation for parent directory, so
 	 * a generation of 0 means "accept any"
 	 */
-	inode = ext4_iget_normal(sb, ino);
+	inode = ext4_iget(sb, ino, EXT4_IGET_HANDLE);
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 	if (generation && inode->i_generation != generation) {

@@ -4350,7 +4341,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * so we can safely mount the rest of the filesystem now.
 	 */
 
-	root = ext4_iget(sb, EXT4_ROOT_INO);
+	root = ext4_iget(sb, EXT4_ROOT_INO, EXT4_IGET_SPECIAL);
 	if (IS_ERR(root)) {
 		ext4_msg(sb, KERN_ERR, "get root inode failed");
 		ret = PTR_ERR(root);

@@ -4618,7 +4609,7 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 	 * happen if we iget() an unused inode, as the subsequent iput()
 	 * will try to delete it.
 	 */
-	journal_inode = ext4_iget(sb, journal_inum);
+	journal_inode = ext4_iget(sb, journal_inum, EXT4_IGET_SPECIAL);
 	if (IS_ERR(journal_inode)) {
 		ext4_msg(sb, KERN_ERR, "no journal found");
 		return NULL;

@@ -5700,7 +5691,7 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
 	if (!qf_inums[type])
 		return -EPERM;
 
-	qf_inode = ext4_iget(sb, qf_inums[type]);
+	qf_inode = ext4_iget(sb, qf_inums[type], EXT4_IGET_SPECIAL);
 	if (IS_ERR(qf_inode)) {
 		ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
 		return PTR_ERR(qf_inode);

fs/ext4/xattr.c +3 -2

index 510c9bb7ce71..7fddd519adae 100644
@@ -384,7 +384,7 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
 	struct inode *inode;
 	int err;
 
-	inode = ext4_iget(parent->i_sb, ea_ino);
+	inode = ext4_iget(parent->i_sb, ea_ino, EXT4_IGET_NORMAL);
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		ext4_error(parent->i_sb,

@@ -1482,7 +1482,8 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 	}
 
 	while (ce) {
-		ea_inode = ext4_iget(inode->i_sb, ce->e_value);
+		ea_inode = ext4_iget(inode->i_sb, ce->e_value,
+				     EXT4_IGET_NORMAL);
 		if (!IS_ERR(ea_inode) &&
 		    !is_bad_inode(ea_inode) &&
 		    (EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL) &&

Signature · 1 line

2.19.1