On 2022/1/4 5:11, Salvatore Bonaccorso wrote:
Hi,
On Mon, Dec 27, 2021 at 04:31:15PM +0100, Greg Kroah-Hartman wrote:
From: Chao Yu chao@kernel.org
commit 5598b24efaf4892741c798b425d543e4bed357a1 upstream.
I've no idea.
I didn't add this line from v1 to v3:
https://lore.kernel.org/lkml/20211211154059.7173-1-chao@kernel.org/T/ https://lore.kernel.org/all/20211212071923.2398-1-chao@kernel.org/T/ https://lore.kernel.org/all/20211212091630.6325-1-chao@kernel.org/T/
Am I missing anything?
Thanks,
As Wenqing Liu reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=215235
- Overview
page fault in f2fs_setxattr() when mount and operate on corrupted image
- Reproduce
tested on kernel 5.16-rc3, 5.15.X under root
- unzip tmp7.zip
- ./single.sh f2fs 7
Sometimes need to run the script several times
- Kernel dump
loop0: detected capacity change from 0 to 131072 F2FS-fs (loop0): Found nat_bits in checkpoint F2FS-fs (loop0): Mounted with checkpoint version = 7548c2ee BUG: unable to handle page fault for address: ffffe47bc7123f48 RIP: 0010:kfree+0x66/0x320 Call Trace: __f2fs_setxattr+0x2aa/0xc00 [f2fs] f2fs_setxattr+0xfa/0x480 [f2fs] __f2fs_set_acl+0x19b/0x330 [f2fs] __vfs_removexattr+0x52/0x70 __vfs_removexattr_locked+0xb1/0x140 vfs_removexattr+0x56/0x100 removexattr+0x57/0x80 path_removexattr+0xa3/0xc0 __x64_sys_removexattr+0x17/0x20 do_syscall_64+0x37/0xb0 entry_SYSCALL_64_after_hwframe+0x44/0xae
The root cause is in __f2fs_setxattr(), we missed to do sanity check on last xattr entry, result in out-of-bound memory access during updating inconsistent xattr data of target inode.
After the fix, it can detect such xattr inconsistency as below:
F2FS-fs (loop11): inode (7) has invalid last xattr entry, entry_size: 60676 F2FS-fs (loop11): inode (8) has corrupted xattr F2FS-fs (loop11): inode (8) has corrupted xattr F2FS-fs (loop11): inode (8) has invalid last xattr entry, entry_size: 47736
Cc: stable@vger.kernel.org Reported-by: Wenqing Liu wenqingliu0120@gmail.com Signed-off-by: Chao Yu chao@kernel.org Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/f2fs/xattr.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
--- a/fs/f2fs/xattr.c +++ b/fs/f2fs/xattr.c @@ -680,8 +680,17 @@ static int __f2fs_setxattr(struct inode } last = here;
- while (!IS_XATTR_LAST_ENTRY(last))
- while (!IS_XATTR_LAST_ENTRY(last)) {
if ((void *)(last) + sizeof(__u32) > last_base_addr ||(void *)XATTR_NEXT_ENTRY(last) > last_base_addr) {f2fs_err(F2FS_I_SB(inode), "inode (%lu) has invalid last xattr entry, entry_size: %zu",inode->i_ino, ENTRY_SIZE(last));set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);error = -EFSCORRUPTED;goto exit;}- }
newsize = XATTR_ALIGN(sizeof(struct f2fs_xattr_entry) + len + size);
It looks this commit while it was applied to several stable series (TTBOMK in 5.15.12, 5.10.89, 5.4.169, 4.19.223 and 4.14.260) it is still missing from mainline, Chao, or anyone else, do you know what happened here?
Regards, Salvatore