* [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero [not found] <20220317172943.2426272-1-lijinlin3@huawei.com> @ 2022-03-17 16:54 ` Li Jinlin 2022-03-21 1:59 ` Zhiqiang Liu 2023-01-18 5:53 ` Theodore Ts'o 0 siblings, 2 replies; 3+ messages in thread From: Li Jinlin @ 2022-03-17 16:54 UTC (permalink / raw) To: tytso; +Cc: linux-ext4, Zhiqiang Liu, linfeilong We got issue as follows: [root@localhost ~]# e2fsck -a img img: recovering journal img: Truncating orphaned inode 188 (uid=0, gid=0, mode=0100666, size=0) img: Truncating orphaned inode 174 (uid=0, gid=0, mode=0100666, size=0) img: clean, 484/128016 files, 118274/512000 blocks [root@localhost ~]# e2fsck -fn img e2fsck 1.46.5 (30-Dec-2021) Pass 1: Checking inodes, blocks, and sizes Inode 174, i_blocks is 2, should be 0. Fix? no Inode 188, i_blocks is 2, should be 0. Fix? no Pass 2: Checking directory structure Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information img: ********** WARNING: Filesystem still has errors ********** img: 484/128016 files (24.6% non-contiguous), 118274/512000 blocks File ACL would be set to zero in release_inode_blocks(), but the blocks count will not be subtract acl blocks, which causes this issue. To slove this issue, subtract acl blocks when setting i_file_acl to zero. Signed-off-by: LiJinlin <lijinlin3@huawei.com> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> --- e2fsck/super.c | 7 +++++-- lib/ext2fs/ext2fs.h | 5 +++++ lib/ext2fs/ext_attr.c | 15 +++++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/e2fsck/super.c b/e2fsck/super.c index 9495e029..715a8dc9 100644 --- a/e2fsck/super.c +++ b/e2fsck/super.c @@ -194,6 +194,7 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, blk64_t blk; errcode_t retval; __u32 count; + __u32 blocks; if (!ext2fs_inode_has_valid_blocks2(fs, EXT2_INODE(inode))) return 0; @@ -238,8 +239,10 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, blk = ext2fs_file_acl_block(fs, EXT2_INODE(inode)); if (blk) { - retval = ext2fs_adjust_ea_refcount3(fs, blk, block_buf, -1, - &count, ino); + retval = ext2fs_adjust_ea_refcount4(fs, blk, block_buf, -1, + &count, ino, &blocks); + if (retval == 0) + ext2fs_iblk_sub_blocks(fs, EXT2_INODE(inode), blocks); if (retval == EXT2_ET_BAD_EA_BLOCK_NUM) { retval = 0; count = 1; diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 68f9c1fe..8ebebf31 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -1289,6 +1289,11 @@ extern errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, char *block_buf, int adjust, __u32 *newcount, ext2_ino_t inum); +extern errcode_t ext2fs_adjust_ea_refcount4(ext2_filsys fs, blk64_t blk, + char *block_buf, + int adjust, __u32 *newcount, + ext2_ino_t inum, + __u32 *blocks); errcode_t ext2fs_xattrs_write(struct ext2_xattr_handle *handle); errcode_t ext2fs_xattrs_read(struct ext2_xattr_handle *handle); errcode_t ext2fs_xattrs_read_inode(struct ext2_xattr_handle *handle, diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c index d36fe68d..1538a53a 100644 --- a/lib/ext2fs/ext_attr.c +++ b/lib/ext2fs/ext_attr.c @@ -237,9 +237,10 @@ errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf) /* * This function adjusts the reference count of the EA block. */ -errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, +errcode_t ext2fs_adjust_ea_refcount4(ext2_filsys fs, blk64_t blk, char *block_buf, int adjust, - __u32 *newcount, ext2_ino_t inum) + __u32 *newcount, ext2_ino_t inum, + __u32 *blocks) { errcode_t retval; struct ext2_ext_attr_header *header; @@ -264,6 +265,8 @@ errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, header->h_refcount += adjust; if (newcount) *newcount = header->h_refcount; + if (blocks) + *blocks = header->h_blocks; retval = ext2fs_write_ext_attr3(fs, blk, block_buf, inum); if (retval) @@ -275,6 +278,14 @@ errout: return retval; } +errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, + char *block_buf, int adjust, + __u32 *newcount, ext2_ino_t inum) +{ + return ext2fs_adjust_ea_refcount4(fs, blk, block_buf, adjust, + newcount, 0, NULL); +} + errcode_t ext2fs_adjust_ea_refcount2(ext2_filsys fs, blk64_t blk, char *block_buf, int adjust, __u32 *newcount) -- 2.27.0 . ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero 2022-03-17 16:54 ` [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero Li Jinlin @ 2022-03-21 1:59 ` Zhiqiang Liu 2023-01-18 5:53 ` Theodore Ts'o 1 sibling, 0 replies; 3+ messages in thread From: Zhiqiang Liu @ 2022-03-21 1:59 UTC (permalink / raw) To: Li Jinlin, tytso; +Cc: linux-ext4, linfeilong For truncating orphaned inode, we should not remove its ACL in release_inode_blocks(), otherwise it may cause safety problems due to loss of acl. diff --git a/e2fsck/super.c b/e2fsck/super.c index 31e2ffb..6249e12 100644 --- a/e2fsck/super.c +++ b/e2fsck/super.c @@ -236,6 +236,10 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, ext2fs_iblk_sub_blocks(fs, EXT2_INODE(inode), pb.truncated_blocks); + /* do not clean up file acl if the inode is truncating type */ + if (inode->i_links_count) + return 0; + blk = ext2fs_file_acl_block(fs, EXT2_INODE(inode)); if (blk) { retval = ext2fs_adjust_ea_refcount3(fs, blk, block_buf, -1, -- 2.27.0 On 2022/3/18 0:54, Li Jinlin wrote: > We got issue as follows: > [root@localhost ~]# e2fsck -a img > img: recovering journal > img: Truncating orphaned inode 188 (uid=0, gid=0, mode=0100666, size=0) > img: Truncating orphaned inode 174 (uid=0, gid=0, mode=0100666, size=0) > img: clean, 484/128016 files, 118274/512000 blocks > [root@localhost ~]# e2fsck -fn img > e2fsck 1.46.5 (30-Dec-2021) > Pass 1: Checking inodes, blocks, and sizes > Inode 174, i_blocks is 2, should be 0. Fix? no > > Inode 188, i_blocks is 2, should be 0. Fix? no > > Pass 2: Checking directory structure > Pass 3: Checking directory connectivity > Pass 4: Checking reference counts > Pass 5: Checking group summary information > > img: ********** WARNING: Filesystem still has errors ********** > > img: 484/128016 files (24.6% non-contiguous), 118274/512000 blocks > > > File ACL would be set to zero in release_inode_blocks(), but the > blocks count will not be subtract acl blocks, which causes this issue. > > To slove this issue, subtract acl blocks when setting i_file_acl to > zero. > > Signed-off-by: LiJinlin <lijinlin3@huawei.com> > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> > --- > e2fsck/super.c | 7 +++++-- > lib/ext2fs/ext2fs.h | 5 +++++ > lib/ext2fs/ext_attr.c | 15 +++++++++++++-- > 3 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/e2fsck/super.c b/e2fsck/super.c > index 9495e029..715a8dc9 100644 > --- a/e2fsck/super.c > +++ b/e2fsck/super.c > @@ -194,6 +194,7 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, > blk64_t blk; > errcode_t retval; > __u32 count; > + __u32 blocks; > > if (!ext2fs_inode_has_valid_blocks2(fs, EXT2_INODE(inode))) > return 0; > @@ -238,8 +239,10 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino, > > blk = ext2fs_file_acl_block(fs, EXT2_INODE(inode)); > if (blk) { > - retval = ext2fs_adjust_ea_refcount3(fs, blk, block_buf, -1, > - &count, ino); > + retval = ext2fs_adjust_ea_refcount4(fs, blk, block_buf, -1, > + &count, ino, &blocks); > + if (retval == 0) > + ext2fs_iblk_sub_blocks(fs, EXT2_INODE(inode), blocks); > if (retval == EXT2_ET_BAD_EA_BLOCK_NUM) { > retval = 0; > count = 1; > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index 68f9c1fe..8ebebf31 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -1289,6 +1289,11 @@ extern errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, > char *block_buf, > int adjust, __u32 *newcount, > ext2_ino_t inum); > +extern errcode_t ext2fs_adjust_ea_refcount4(ext2_filsys fs, blk64_t blk, > + char *block_buf, > + int adjust, __u32 *newcount, > + ext2_ino_t inum, > + __u32 *blocks); > errcode_t ext2fs_xattrs_write(struct ext2_xattr_handle *handle); > errcode_t ext2fs_xattrs_read(struct ext2_xattr_handle *handle); > errcode_t ext2fs_xattrs_read_inode(struct ext2_xattr_handle *handle, > diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c > index d36fe68d..1538a53a 100644 > --- a/lib/ext2fs/ext_attr.c > +++ b/lib/ext2fs/ext_attr.c > @@ -237,9 +237,10 @@ errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf) > /* > * This function adjusts the reference count of the EA block. > */ > -errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, > +errcode_t ext2fs_adjust_ea_refcount4(ext2_filsys fs, blk64_t blk, > char *block_buf, int adjust, > - __u32 *newcount, ext2_ino_t inum) > + __u32 *newcount, ext2_ino_t inum, > + __u32 *blocks) > { > errcode_t retval; > struct ext2_ext_attr_header *header; > @@ -264,6 +265,8 @@ errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, > header->h_refcount += adjust; > if (newcount) > *newcount = header->h_refcount; > + if (blocks) > + *blocks = header->h_blocks; > > retval = ext2fs_write_ext_attr3(fs, blk, block_buf, inum); > if (retval) > @@ -275,6 +278,14 @@ errout: > return retval; > } > > +errcode_t ext2fs_adjust_ea_refcount3(ext2_filsys fs, blk64_t blk, > + char *block_buf, int adjust, > + __u32 *newcount, ext2_ino_t inum) > +{ > + return ext2fs_adjust_ea_refcount4(fs, blk, block_buf, adjust, > + newcount, 0, NULL); > +} > + > errcode_t ext2fs_adjust_ea_refcount2(ext2_filsys fs, blk64_t blk, > char *block_buf, int adjust, > __u32 *newcount) > ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero 2022-03-17 16:54 ` [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero Li Jinlin 2022-03-21 1:59 ` Zhiqiang Liu @ 2023-01-18 5:53 ` Theodore Ts'o 1 sibling, 0 replies; 3+ messages in thread From: Theodore Ts'o @ 2023-01-18 5:53 UTC (permalink / raw) To: Li Jinlin; +Cc: linux-ext4, Zhiqiang Liu, linfeilong This patch is not correct. We don't need to create a new ext2fs_adjust_ea_refcount4 to return the h_blocks field in the extended attribute block, since that only thing that we support is a single xattr block. The real issue, as Zhiqiang Liu pointed out in [1], is that we should not be clearing the i_file_acl block is the inode is only being truncated, and not being unlinked. [1] https://lore.kernel.org/all/ed518b11-3c38-1c1f-a75d-3293c91f17d4@huawei.com/ - Ted ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-18 5:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220317172943.2426272-1-lijinlin3@huawei.com>
2022-03-17 16:54 ` [PATCH] e2fsck: subtract acl blocks when setting i_file_acl to zero Li Jinlin
2022-03-21 1:59 ` Zhiqiang Liu
2023-01-18 5:53 ` Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).