From f8de29d5cd226ec86aac3500b70eaf203cc1ae0b Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Fri, 12 Feb 2016 16:12:40 -0700 Subject: [PATCH] LU-7581 ldiskfs: RHEL7.2 fix wrong EA inode backpointer check Port http://review.whamcloud.com/17675 for RHEL7.2: EA inode is linked back to the parent inode using i_mtime.tv_sec filed. An inode number bigger 2G gets mangled due to sign bit extension over the high bits of tv_sec. It causes parent backpointer checks to fail. Add an explicit integer type conversion to ignore high bits of i_mtime.tv_sec. Alexander Zarochentsev Fix other code style issues and comments for upstream kernel. Signed-off-by: Andreas Dilger Change-Id: I7d402f000cde5ffb1ededf7a80276538e4465757 Reviewed-on: http://review.whamcloud.com/18436 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Bob Glossman Reviewed-by: Yang Sheng Reviewed-by: Oleg Drokin --- .../patches/rhel7.2/ext4-large-eas.patch | 60 ++++++++++++---------- .../patches/sles12/ext4-large-eas.patch | 51 +++++++++--------- 2 files changed, 58 insertions(+), 53 deletions(-) diff --git a/ldiskfs/kernel_patches/patches/rhel7.2/ext4-large-eas.patch b/ldiskfs/kernel_patches/patches/rhel7.2/ext4-large-eas.patch index 03bdf17..cf59bc4 100644 --- a/ldiskfs/kernel_patches/patches/rhel7.2/ext4-large-eas.patch +++ b/ldiskfs/kernel_patches/patches/rhel7.2/ext4-large-eas.patch @@ -20,7 +20,7 @@ Index: linux-stage/fs/ext4/ext4.h #define EXT4_MMP_MAX_CHECK_INTERVAL 300UL /* -+ * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb ++ * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1MB + * This limit is arbitrary, but is reasonable for the xattr API. + */ +#define EXT4_XATTR_MAX_LARGE_EA_SIZE (1024 * 1024) @@ -81,17 +81,12 @@ Index: linux-stage/fs/ext4/inode.c if (IS_ERR(handle)) { ext4_std_error(inode->i_sb, PTR_ERR(handle)); /* -@@ -249,9 +249,36 @@ void ext4_evict_inode(struct inode *inod - sb_end_intwrite(inode->i_sb); - goto no_delete; - } -- +@@ -252,6 +252,32 @@ void ext4_evict_inode(struct inode *inod + if (IS_SYNC(inode)) ext4_handle_sync(handle); + -+ /* -+ * Delete xattr inode before deleting the main inode. -+ */ ++ /* Delete xattr inode before deleting the main inode. */ + err = ext4_xattr_delete_inode(handle, inode, &lea_ino_array); + if (err) { + ext4_warning(inode->i_sb, @@ -119,11 +114,8 @@ Index: linux-stage/fs/ext4/inode.c inode->i_size = 0; err = ext4_mark_inode_dirty(handle, inode); if (err) { -@@ -306,8 +333,12 @@ void ext4_evict_inode(struct inode *inod - ext4_clear_inode(inode); - else +@@ -308,6 +335,9 @@ void ext4_evict_inode(struct inode *inod ext4_free_inode(handle, inode); -+ ext4_journal_stop(handle); sb_end_intwrite(inode->i_sb); + @@ -132,7 +124,7 @@ Index: linux-stage/fs/ext4/inode.c return; no_delete: ext4_clear_inode(inode); /* We must guarantee clearing of inode... */ -@@ -4681,7 +4712,7 @@ static int ext4_index_trans_blocks(struc +@@ -4681,7 +4711,7 @@ static int ext4_index_trans_blocks(struc * * Also account for superblock, inode, quota and xattr blocks */ @@ -183,7 +175,7 @@ Index: linux-stage/fs/ext4/xattr.c { struct ext4_xattr_entry *entry; size_t name_len; -@@ -265,11 +273,104 @@ ext4_xattr_find_entry(struct ext4_xattr_ +@@ -265,11 +273,109 @@ ext4_xattr_find_entry(struct ext4_xattr_ break; } *pentry = entry; @@ -196,8 +188,7 @@ Index: linux-stage/fs/ext4/xattr.c +/* + * Read the EA value from an inode. + */ -+static int -+ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size) ++static int ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size) +{ + unsigned long block = 0; + struct buffer_head *bh = NULL; @@ -230,7 +221,14 @@ Index: linux-stage/fs/ext4/xattr.c + return err; +} + -+struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, int *err) ++/* ++ * Fetch the xattr inode from disk. ++ * ++ * The xattr inode stores the parent inode number and generation so that ++ * the kernel and e2fsck can verify the xattr inode is valid upon access. ++ */ ++struct inode *ext4_xattr_inode_iget(struct inode *parent, ++ unsigned long ea_ino, int *err) +{ + struct inode *ea_inode = NULL; + @@ -243,7 +241,7 @@ Index: linux-stage/fs/ext4/xattr.c + return NULL; + } + -+ if (ea_inode->i_xattr_inode_parent != parent->i_ino || ++ if (EXT4_XATTR_INODE_GET_PARENT(ea_inode) != parent->i_ino || + ea_inode->i_generation != parent->i_generation) { + ext4_error(parent->i_sb, "Backpointer from EA inode %lu " + "to parent invalid.", ea_ino); @@ -269,9 +267,8 @@ Index: linux-stage/fs/ext4/xattr.c +/* + * Read the value from the EA inode. + */ -+static int -+ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer, -+ size_t *size) ++static int ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, ++ void *buffer, size_t *size) +{ + struct inode *ea_inode = NULL; + int err; @@ -458,7 +455,7 @@ Index: linux-stage/fs/ext4/xattr.c + * A back-pointer from EA inode to parent inode will be useful + * for e2fsck. + */ -+ ea_inode->i_xattr_inode_parent = inode->i_ino; ++ EXT4_XATTR_INODE_SET_PARENT(ea_inode, inode->i_ino); + unlock_new_inode(ea_inode); + } + @@ -611,7 +608,7 @@ Index: linux-stage/fs/ext4/xattr.c bs->s.here = bs->s.first; error = ext4_xattr_find_entry(&bs->s.here, i->name_index, - i->name, bs->bh->b_size, 1); -+ i->name, bs->bh->b_size, 1, inode); ++ i->name, bs->bh->b_size, 1, inode); if (error && error != -ENODATA) goto cleanup; bs->s.not_found = error; @@ -989,11 +986,22 @@ Index: linux-stage/fs/ext4/xattr.h __le32 e_value_size; /* size of attribute value */ __le32 e_hash; /* hash value of name and value */ char e_name[0]; /* attribute name */ -@@ -67,6 +67,15 @@ struct ext4_xattr_entry { +@@ -67,6 +67,26 @@ struct ext4_xattr_entry { EXT4_I(inode)->i_extra_isize)) #define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1)) -+#define i_xattr_inode_parent i_mtime.tv_sec ++/* ++ * Link EA inode back to parent one using i_mtime field. ++ * Extra integer type conversion added to ignore higher ++ * bits in i_mtime.tv_sec which might be set by ext4_get() ++ */ ++#define EXT4_XATTR_INODE_SET_PARENT(inode, inum) \ ++do { \ ++ (inode)->i_mtime.tv_sec = inum; \ ++} while(0) ++ ++#define EXT4_XATTR_INODE_GET_PARENT(inode) \ ++ ((__u32)(inode)->i_mtime.tv_sec) + +/* + * The minimum size of EA value when you start storing it in an external inode diff --git a/ldiskfs/kernel_patches/patches/sles12/ext4-large-eas.patch b/ldiskfs/kernel_patches/patches/sles12/ext4-large-eas.patch index baf0b57..6c94182 100644 --- a/ldiskfs/kernel_patches/patches/sles12/ext4-large-eas.patch +++ b/ldiskfs/kernel_patches/patches/sles12/ext4-large-eas.patch @@ -20,7 +20,7 @@ Index: linux-stage/fs/ext4/ext4.h #define EXT4_MMP_MAX_CHECK_INTERVAL 300UL /* -+ * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb ++ * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1MB + * This limit is arbitrary, but is reasonable for the xattr API. + */ +#define EXT4_XATTR_MAX_LARGE_EA_SIZE (1024 * 1024) @@ -81,17 +81,12 @@ Index: linux-stage/fs/ext4/inode.c if (IS_ERR(handle)) { ext4_std_error(inode->i_sb, PTR_ERR(handle)); /* -@@ -248,9 +248,36 @@ void ext4_evict_inode(struct inode *inod - sb_end_intwrite(inode->i_sb); - goto no_delete; - } -- +@@ -251,6 +251,32 @@ void ext4_evict_inode(struct inode *inod + if (IS_SYNC(inode)) ext4_handle_sync(handle); + -+ /* -+ * Delete xattr inode before deleting the main inode. -+ */ ++ /* Delete xattr inode before deleting the main inode. */ + err = ext4_xattr_delete_inode(handle, inode, &lea_ino_array); + if (err) { + ext4_warning(inode->i_sb, @@ -119,11 +114,8 @@ Index: linux-stage/fs/ext4/inode.c inode->i_size = 0; err = ext4_mark_inode_dirty(handle, inode); if (err) { -@@ -305,8 +332,12 @@ void ext4_evict_inode(struct inode *inod - ext4_clear_inode(inode); - else +@@ -307,6 +334,9 @@ void ext4_evict_inode(struct inode *inod ext4_free_inode(handle, inode); -+ ext4_journal_stop(handle); sb_end_intwrite(inode->i_sb); + @@ -132,7 +124,7 @@ Index: linux-stage/fs/ext4/inode.c return; no_delete: ext4_clear_inode(inode); /* We must guarantee clearing of inode... */ -@@ -4777,7 +4808,7 @@ static int ext4_index_trans_blocks(struc +@@ -4777,7 +4807,7 @@ static int ext4_index_trans_blocks(struc * * Also account for superblock, inode, quota and xattr blocks */ @@ -175,7 +167,7 @@ Index: linux-stage/fs/ext4/xattr.c { struct ext4_xattr_entry *entry; size_t name_len; -@@ -265,11 +272,104 @@ ext4_xattr_find_entry(struct ext4_xattr_ +@@ -265,11 +272,109 @@ ext4_xattr_find_entry(struct ext4_xattr_ break; } *pentry = entry; @@ -188,8 +180,7 @@ Index: linux-stage/fs/ext4/xattr.c +/* + * Read the EA value from an inode. + */ -+static int -+ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size) ++static int ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size) +{ + unsigned long block = 0; + struct buffer_head *bh = NULL; @@ -222,7 +213,14 @@ Index: linux-stage/fs/ext4/xattr.c + return err; +} + -+struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, int *err) ++/* ++ * Fetch the xattr inode from disk. ++ * ++ * The xattr inode stores the parent inode number and generation so that ++ * the kernel and e2fsck can verify the xattr inode is valid upon access. ++ */ ++struct inode *ext4_xattr_inode_iget(struct inode *parent, ++ unsigned long ea_ino, int *err) +{ + struct inode *ea_inode = NULL; + @@ -261,9 +259,8 @@ Index: linux-stage/fs/ext4/xattr.c +/* + * Read the value from the EA inode. + */ -+static int -+ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer, -+ size_t *size) ++static int ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, ++ void *buffer, size_t *size) +{ + struct inode *ea_inode = NULL; + int err; @@ -602,7 +599,7 @@ Index: linux-stage/fs/ext4/xattr.c bs->s.here = bs->s.first; error = ext4_xattr_find_entry(&bs->s.here, i->name_index, - i->name, bs->bh->b_size, 1); -+ i->name, bs->bh->b_size, 1, inode); ++ i->name, bs->bh->b_size, 1, inode); if (error && error != -ENODATA) goto cleanup; bs->s.not_found = error; @@ -989,13 +986,13 @@ Index: linux-stage/fs/ext4/xattr.h + * Extra integer type conversion added to ignore higher + * bits in i_mtime.tv_sec which might be set by ext4_get() + */ -+#define EXT4_XATTR_INODE_SET_PARENT(inode, inum) \ -+do { \ -+ (inode)->i_mtime.tv_sec = inum; \ ++#define EXT4_XATTR_INODE_SET_PARENT(inode, inum) \ ++do { \ ++ (inode)->i_mtime.tv_sec = inum; \ +} while(0) + -+#define EXT4_XATTR_INODE_GET_PARENT(inode) \ -+((__u32)(inode)->i_mtime.tv_sec) ++#define EXT4_XATTR_INODE_GET_PARENT(inode) \ ++ ((__u32)(inode)->i_mtime.tv_sec) + +/* + * The minimum size of EA value when you start storing it in an external inode -- 1.8.3.1