[f2fs-dev] [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
between the original inode and COW inode. When aborting atomic write and
writeback occur simultaneously, invalid data can be written to original
inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
To prevent the problem, let's truncate all pages before clearing the flag
Atomic write thread Writeback thread
f2fs_abort_atomic_write
clear_inode_flag(inode, FI_ATOMIC_FILE)
__writeback_single_inode
do_writepages
f2fs_do_write_data_page
- use dn of original inode
truncate_inode_pages_final
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: sta...@vger.kernel.org #v5.19+
Reviewed-by: Sungjong Seo
Reviewed-by: Yeongjin Gil
Signed-off-by: Sunmin Jeong
---
fs/f2fs/segment.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 7901ede58113..7e47b8054413 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool
clean)
if (!f2fs_is_atomic_file(inode))
return;
+ if (clean)
+ truncate_inode_pages_final(inode->i_mapping);
+
release_atomic_write_cnt(inode);
clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
clear_inode_flag(inode, FI_ATOMIC_REPLACE);
@@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool
clean)
F2FS_I(inode)->atomic_write_task = NULL;
if (clean) {
- truncate_inode_pages_final(inode->i_mapping);
f2fs_i_size_write(inode, fi->original_i_size);
fi->original_i_size = 0;
}
--
2.25.1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
In f2fs_update_inode, i_size of the atomic file isn't updated until
FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
after the writeback of the inode, i_size of the raw inode will not be
updated. It can cause the atomicity corruption due to a mismatch between
old file size and new data.
To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
Atomic write thread Writeback thread
__writeback_single_inode
write_inode
f2fs_update_inode
- skip i_size update
f2fs_ioc_commit_atomic_write
f2fs_commit_atomic_write
set_inode_flag(inode, FI_ATOMIC_COMMITTED)
f2fs_do_sync_file
f2fs_fsync_node_pages
- skip f2fs_update_inode since the inode is clean
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Cc: sta...@vger.kernel.org #v5.19+
Reviewed-by: Sungjong Seo
Reviewed-by: Yeongjin Gil
Signed-off-by: Sunmin Jeong
---
fs/f2fs/f2fs.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 543898482f8b..a000cb024dbe 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3039,6 +3039,7 @@ static inline void __mark_inode_dirty_flag(struct inode
*inode,
case FI_INLINE_DOTS:
case FI_PIN_FILE:
case FI_COMPRESS_RELEASED:
+ case FI_ATOMIC_COMMITTED:
f2fs_mark_inode_dirty_sync(inode, true);
}
}
--
2.25.1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in f2fs_filemap_fault
#syz test git://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git wip On 2024/1/15 17:12, syzbot wrote: Hello, syzbot found the following issue on: HEAD commit:052d534373b7 Merge tag 'exfat-for-6.8-rc1' of git://git.ke.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=14bb9913e8 kernel config: https://syzkaller.appspot.com/x/.config?x=490fc2f9d4ae426c dashboard link: https://syzkaller.appspot.com/bug?extid=763afad57075d3f862f2 compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15d9fbcbe8 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1422d5ebe8 Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/51de89c7a81e/disk-052d5343.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/7e03b92536a3/vmlinux-052d5343.xz kernel image: https://storage.googleapis.com/syzbot-assets/3d91124eb5ff/bzImage-052d5343.xz mounted in repro #1: https://storage.googleapis.com/syzbot-assets/f67519526788/mount_0.gz mounted in repro #2: https://storage.googleapis.com/syzbot-assets/7e871268c842/mount_7.gz IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+763afad57075d3f86...@syzkaller.appspotmail.com == BUG: KASAN: slab-use-after-free in f2fs_filemap_fault+0xd1/0x2c0 fs/f2fs/file.c:49 Read of size 8 at addr 88807bb22680 by task syz-executor184/5058 CPU: 0 PID: 5058 Comm: syz-executor184 Not tainted 6.7.0-syzkaller-09928-g052d534373b7 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023 Call Trace: __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106 print_address_description mm/kasan/report.c:377 [inline] print_report+0x163/0x540 mm/kasan/report.c:488 kasan_report+0x142/0x170 mm/kasan/report.c:601 f2fs_filemap_fault+0xd1/0x2c0 fs/f2fs/file.c:49 __do_fault+0x131/0x450 mm/memory.c:4376 do_shared_fault mm/memory.c:4798 [inline] do_fault mm/memory.c:4872 [inline] do_pte_missing mm/memory.c:3745 [inline] handle_pte_fault mm/memory.c:5144 [inline] __handle_mm_fault+0x23b7/0x72b0 mm/memory.c:5285 handle_mm_fault+0x27e/0x770 mm/memory.c:5450 do_user_addr_fault arch/x86/mm/fault.c:1364 [inline] handle_page_fault arch/x86/mm/fault.c:1507 [inline] exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1563 asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570 RIP: 0033:0x7f808e742b50 Code: 20 bf 02 00 00 00 e8 9f 24 04 00 48 83 f8 ff 0f 84 08 fa ff ff 48 89 05 e6 65 0c 00 e9 fc f9 ff ff 66 0f 1f 84 00 00 00 00 00 04 25 40 00 00 20 66 32 66 73 c6 04 25 44 00 00 20 00 e9 0f fa RSP: 002b:7f808e735170 EFLAGS: 00010246 RAX: RBX: 7f808e80b6e8 RCX: 7f808e784fe9 RDX: 86d0f56bab720225 RSI: RDI: 7f808e7355a0 RBP: 7f808e80b6e0 R08: R09: 7f808e7356c0 R10: 7f808e80b6e0 R11: 0246 R12: 7f808e80b6ec R13: R14: 7fff41e6fc30 R15: 7fff41e6fd18 Allocated by task 5058: kasan_save_stack mm/kasan/common.c:47 [inline] kasan_save_track+0x3f/0x70 mm/kasan/common.c:68 unpoison_slab_object mm/kasan/common.c:314 [inline] __kasan_slab_alloc+0x66/0x70 mm/kasan/common.c:340 kasan_slab_alloc include/linux/kasan.h:201 [inline] slab_post_alloc_hook mm/slub.c:3813 [inline] slab_alloc_node mm/slub.c:3860 [inline] kmem_cache_alloc+0x16f/0x340 mm/slub.c:3867 vm_area_alloc+0x24/0x1d0 kernel/fork.c:465 mmap_region+0xbd8/0x1f90 mm/mmap.c:2804 do_mmap+0x76b/0xde0 mm/mmap.c:1379 vm_mmap_pgoff+0x1e2/0x420 mm/util.c:556 ksys_mmap_pgoff+0x4ff/0x6d0 mm/mmap.c:1425 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x63/0x6b Freed by task 5064: kasan_save_stack mm/kasan/common.c:47 [inline] kasan_save_track+0x3f/0x70 mm/kasan/common.c:68 kasan_save_free_info+0x4e/0x60 mm/kasan/generic.c:634 poison_slab_object+0xa6/0xe0 mm/kasan/common.c:241 __kasan_slab_free+0x34/0x60 mm/kasan/common.c:257 kasan_slab_free include/linux/kasan.h:184 [inline] slab_free_hook mm/slub.c:2121 [inline] slab_free mm/slub.c:4299 [inline] kmem_cache_free+0x102/0x2a0 mm/slub.c:4363 rcu_do_batch kernel/rcu/tree.c:2158 [inline] rcu_core+0xad8/0x17c0 kernel/rcu/tree.c:2433 __do_softirq+0x2b8/0x939 kernel/softirq.c:553 Last potentially related work creation: kasan_save_stack+0x3f/0x60 mm/kasan/common.c:47 __kasan_record_aux_stack+0xae/0x100 mm/kasan/generic.c:580 __call_rcu_common kernel/rcu/tree.c:2683 [inline] call_rcu+0x167/0xa80 kernel/rcu/tree.c:2797 remove_vma mm/mmap.c:148 [inline] remove_mt mm/mmap.c:2283 [inline] do_vmi_align_munmap+0x159d/0x1930 mm/mmap.c:2629 do_vmi_munmap+0x24d/0x2d0 mm/mmap.c:2693
Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in f2fs_filemap_fault
Hello, syzbot has tested the proposed patch and the reproducer did not trigger any issue: Reported-and-tested-by: syzbot+763afad57075d3f86...@syzkaller.appspotmail.com Tested on: commit: 51fc665a f2fs: fix to avoid use-after-free issue in f2.. git tree: git://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git wip console output: https://syzkaller.appspot.com/x/log.txt?x=113bf9fa18 kernel config: https://syzkaller.appspot.com/x/.config?x=1fcc0a6ff51770e5 dashboard link: https://syzkaller.appspot.com/bug?extid=763afad57075d3f862f2 compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 Note: no patches were applied. Note: testing is done by a robot and is best-effort only. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/2] f2fs: mark inode dirty for FI_ATOMIC_COMMITTED flag
Reviewed-by: Daeho Jeong
On Wed, Mar 13, 2024 at 4:37 AM Sunmin Jeong wrote:
>
> In f2fs_update_inode, i_size of the atomic file isn't updated until
> FI_ATOMIC_COMMITTED flag is set. When committing atomic write right
> after the writeback of the inode, i_size of the raw inode will not be
> updated. It can cause the atomicity corruption due to a mismatch between
> old file size and new data.
>
> To prevent the problem, let's mark inode dirty for FI_ATOMIC_COMMITTED
>
> Atomic write thread Writeback thread
> __writeback_single_inode
> write_inode
> f2fs_update_inode
> - skip i_size update
> f2fs_ioc_commit_atomic_write
> f2fs_commit_atomic_write
> set_inode_flag(inode, FI_ATOMIC_COMMITTED)
> f2fs_do_sync_file
> f2fs_fsync_node_pages
> - skip f2fs_update_inode since the inode is clean
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: sta...@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo
> Reviewed-by: Yeongjin Gil
> Signed-off-by: Sunmin Jeong
> ---
> fs/f2fs/f2fs.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 543898482f8b..a000cb024dbe 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3039,6 +3039,7 @@ static inline void __mark_inode_dirty_flag(struct inode
> *inode,
> case FI_INLINE_DOTS:
> case FI_PIN_FILE:
> case FI_COMPRESS_RELEASED:
> + case FI_ATOMIC_COMMITTED:
> f2fs_mark_inode_dirty_sync(inode, true);
> }
> }
> --
> 2.25.1
>
>
>
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/2] f2fs: truncate page cache before clearing flags when aborting atomic write
Reviewed-by: Daeho Jeong
On Wed, Mar 13, 2024 at 4:29 AM Sunmin Jeong wrote:
>
> In f2fs_do_write_data_page, FI_ATOMIC_FILE flag selects the target inode
> between the original inode and COW inode. When aborting atomic write and
> writeback occur simultaneously, invalid data can be written to original
> inode if the FI_ATOMIC_FILE flag is cleared meanwhile.
>
> To prevent the problem, let's truncate all pages before clearing the flag
>
> Atomic write thread Writeback thread
> f2fs_abort_atomic_write
> clear_inode_flag(inode, FI_ATOMIC_FILE)
> __writeback_single_inode
> do_writepages
> f2fs_do_write_data_page
> - use dn of original inode
> truncate_inode_pages_final
>
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Cc: sta...@vger.kernel.org #v5.19+
> Reviewed-by: Sungjong Seo
> Reviewed-by: Yeongjin Gil
> Signed-off-by: Sunmin Jeong
> ---
> fs/f2fs/segment.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 7901ede58113..7e47b8054413 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -192,6 +192,9 @@ void f2fs_abort_atomic_write(struct inode *inode, bool
> clean)
> if (!f2fs_is_atomic_file(inode))
> return;
>
> + if (clean)
> + truncate_inode_pages_final(inode->i_mapping);
> +
> release_atomic_write_cnt(inode);
> clear_inode_flag(inode, FI_ATOMIC_COMMITTED);
> clear_inode_flag(inode, FI_ATOMIC_REPLACE);
> @@ -201,7 +204,6 @@ void f2fs_abort_atomic_write(struct inode *inode, bool
> clean)
> F2FS_I(inode)->atomic_write_task = NULL;
>
> if (clean) {
> - truncate_inode_pages_final(inode->i_mapping);
> f2fs_i_size_write(inode, fi->original_i_size);
> fi->original_i_size = 0;
> }
> --
> 2.25.1
>
>
>
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: fix to avoid use-after-free issue in f2fs_filemap_fault
syzbot reports a f2fs bug as below:
BUG: KASAN: slab-use-after-free in f2fs_filemap_fault+0xd1/0x2c0
fs/f2fs/file.c:49
Read of size 8 at addr 88807bb22680 by task syz-executor184/5058
CPU: 0 PID: 5058 Comm: syz-executor184 Not tainted
6.7.0-syzkaller-09928-g052d534373b7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
11/17/2023
Call Trace:
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
print_address_description mm/kasan/report.c:377 [inline]
print_report+0x163/0x540 mm/kasan/report.c:488
kasan_report+0x142/0x170 mm/kasan/report.c:601
f2fs_filemap_fault+0xd1/0x2c0 fs/f2fs/file.c:49
__do_fault+0x131/0x450 mm/memory.c:4376
do_shared_fault mm/memory.c:4798 [inline]
do_fault mm/memory.c:4872 [inline]
do_pte_missing mm/memory.c:3745 [inline]
handle_pte_fault mm/memory.c:5144 [inline]
__handle_mm_fault+0x23b7/0x72b0 mm/memory.c:5285
handle_mm_fault+0x27e/0x770 mm/memory.c:5450
do_user_addr_fault arch/x86/mm/fault.c:1364 [inline]
handle_page_fault arch/x86/mm/fault.c:1507 [inline]
exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1563
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
The root cause is: in f2fs_filemap_fault(), vmf->vma may be not alive after
filemap_fault(), so it may cause use-after-free issue when accessing
vmf->vma->vm_flags in trace_f2fs_filemap_fault(). So it needs to keep vm_flags
in separated temporary variable for tracepoint use.
Fixes: 87f3afd366f7 ("f2fs: add tracepoint for f2fs_vm_page_mkwrite()")
Reported-and-tested-by: syzbot+763afad57075d3f86...@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/lkml/e8222b060f00d...@google.com
Cc: Ed Tsai
Cc: Hillf Danton
Signed-off-by: Chao Yu
---
fs/f2fs/file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index a47c57e813bb..c19e55a3e50e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -39,6 +39,7 @@
static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
{
struct inode *inode = file_inode(vmf->vma->vm_file);
+ vm_flags_t flags = vmf->vma->vm_flags;
vm_fault_t ret;
ret = filemap_fault(vmf);
@@ -46,7 +47,7 @@ static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
f2fs_update_iostat(F2FS_I_SB(inode), inode,
APP_MAPPED_READ_IO, F2FS_BLKSIZE);
- trace_f2fs_filemap_fault(inode, vmf->pgoff, vmf->vma->vm_flags, ret);
+ trace_f2fs_filemap_fault(inode, vmf->pgoff, flags, ret);
return ret;
}
--
2.40.1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in f2fs_filemap_fault
On 2024/3/13 9:31, Jaegeuk Kim wrote:
On 03/12, Ed Tsai (蔡宗軒) wrote:
On Mon, 2024-01-15 at 20:05 +0800, Hillf Danton wrote:
...
--- x/fs/f2fs/file.c
+++ y/fs/f2fs/file.c
@@ -39,6 +39,7 @@
static vm_fault_t f2fs_filemap_fault(struct vm_fault *vmf)
{
struct inode *inode = file_inode(vmf->vma->vm_file);
+ vm_flags_t flags = vmf->vma->vm_flags;
vm_fault_t ret;
ret = filemap_fault(vmf);
@@ -46,7 +47,7 @@ static vm_fault_t f2fs_filemap_fault(str
f2fs_update_iostat(F2FS_I_SB(inode), inode,
APP_MAPPED_READ_IO,
F2FS_BLKSIZE);
- trace_f2fs_filemap_fault(inode, vmf->pgoff, vmf->vma-
vm_flags, ret);
+ trace_f2fs_filemap_fault(inode, vmf->pgoff, flags, ret);
return ret;
}
--
Hi Jaegeuk,
We recently encountered this slabe-use-after-free issue in KASAN as
well. Could you please review the patch above and merge it into f2fs?
Where is the patch?
Hi, all,
I'd like to fix this issue in 6.9-rc1, so I submitted a formal patch based on
above code, and the patch has been tested by syzbot.
https://lore.kernel.org/linux-f2fs-devel/20240314020528.3051533-1-c...@kernel.org
Hillf, may I change author of the patch to you? :)
Thanks,
Best,
Ed
==
[29195.369964][T31720] BUG: KASAN: slab-use-after-free in
f2fs_filemap_fault+0x50/0xe0
[29195.370971][T31720] Read at addr f780454ebde0 by task AsyncTask
#11/31720
[29195.371881][T31720] Pointer tag: [f7], memory tag: [f1]
[29195.372549][T31720]
[29195.372838][T31720] CPU: 2 PID: 31720 Comm: AsyncTask #11 Tainted:
GW OE 6.6.17-android15-0-gcb5ba718a525 #1
[29195.374862][T31720] Call trace:
[29195.375268][T31720] dump_backtrace+0xec/0x138
[29195.375848][T31720] show_stack+0x18/0x24
[29195.376365][T31720] dump_stack_lvl+0x50/0x6c
[29195.376943][T31720] print_report+0x1b0/0x714
[29195.377520][T31720] kasan_report+0xc4/0x124
[29195.378076][T31720] __do_kernel_fault+0xb8/0x26c
[29195.378694][T31720] do_bad_area+0x30/0xdc
[29195.379226][T31720] do_tag_check_fault+0x20/0x34
[29195.379834][T31720] do_mem_abort+0x58/0x104
[29195.380388][T31720] el1_abort+0x3c/0x5c
[29195.380899][T31720] el1h_64_sync_handler+0x54/0x90
[29195.381529][T31720] el1h_64_sync+0x68/0x6c
[29195.382069][T31720] f2fs_filemap_fault+0x50/0xe0
[29195.382678][T31720] __do_fault+0xc8/0xfc
[29195.383209][T31720] handle_mm_fault+0xb44/0x10c4
[29195.383816][T31720] do_page_fault+0x294/0x48c
[29195.384395][T31720] do_translation_fault+0x38/0x54
[29195.385023][T31720] do_mem_abort+0x58/0x104
[29195.385577][T31720] el0_da+0x44/0x78
[29195.386057][T31720] el0t_64_sync_handler+0x98/0xbc
[29195.386688][T31720] el0t_64_sync+0x1a8/0x1ac
[29195.387249][T31720]
[29195.387534][T31720] Allocated by task 14784:
[29195.388085][T31720] kasan_save_stack+0x40/0x70
[29195.388672][T31720] save_stack_info+0x34/0x128
[29195.389259][T31720] kasan_save_alloc_info+0x14/0x20
[29195.389901][T31720] __kasan_slab_alloc+0x168/0x174
[29195.390530][T31720] slab_post_alloc_hook+0x88/0x3a4
[29195.391168][T31720] kmem_cache_alloc+0x18c/0x2c8
[29195.391771][T31720] vm_area_alloc+0x2c/0xe8
[29195.392327][T31720] mmap_region+0x440/0xa94
[29195.392888][T31720] do_mmap+0x3d0/0x524
[29195.393399][T31720] vm_mmap_pgoff+0x1a0/0x1f8
[29195.393980][T31720] ksys_mmap_pgoff+0x78/0xf4
[29195.394557][T31720] __arm64_sys_mmap+0x34/0x44
[29195.395138][T31720] invoke_syscall+0x58/0x114
[29195.395727][T31720] el0_svc_common+0x80/0xe0
[29195.396292][T31720] do_el0_svc+0x1c/0x28
[29195.396812][T31720] el0_svc+0x38/0x68
[29195.397302][T31720] el0t_64_sync_handler+0x68/0xbc
[29195.397932][T31720] el0t_64_sync+0x1a8/0x1ac
[29195.398492][T31720]
[29195.398778][T31720] Freed by task 0:
[29195.399240][T31720] kasan_save_stack+0x40/0x70
[29195.399825][T31720] save_stack_info+0x34/0x128
[29195.400412][T31720] kasan_save_free_info+0x18/0x28
[29195.401043][T31720] kasan_slab_free+0x254/0x25c
[29195.401682][T31720] __kasan_slab_free+0x10/0x20
[29195.402278][T31720] slab_free_freelist_hook+0x174/0x1e0
[29195.402961][T31720] kmem_cache_free+0xc4/0x348
[29195.403544][T31720] __vm_area_free+0x84/0xa4
[29195.404103][T31720] vm_area_free_rcu_cb+0x10/0x20
[29195.404719][T31720] rcu_do_batch+0x214/0x720
[29195.405284][T31720] rcu_core+0x1b0/0x408
[29195.405800][T31720] rcu_core_si+0x10/0x20
[29195.406348][T31720] __do_softirq+0x120/0x3f4
[29195.406907][T31720]
[29195.407191][T31720] The buggy address belongs to the object at
ff80454ebdc0
[29195.407191][T31720] which belongs to the cache vm_area_struct of
size 176
[29195.408978][T31720] The buggy address is located 32 bytes inside of
[29195.408978][T31720] 176-byte region [ff80454ebdc0,
ff80454ebe70)
[29195.410625][T31720]
[29195.410911][T31720] The buggy address belongs to the physical page:
[29195.411709][T31720] page:58f0f2f1 refcount:1 mapcount:0
Re: [f2fs-dev] [PATCH v13 2/9] f2fs: Simplify the handling of cached insensitive names
Eugen Hristev writes:
> +void f2fs_free_casefolded_name(struct f2fs_filename *fname)
> +{
> + unsigned char *buf = (unsigned char *)fname->cf_name.name;
> +
> + kmem_cache_free(f2fs_cf_name_slab, buf);
> + fname->cf_name.name = NULL;
In my previous review, I mentioned you could drop the "if (buf)" check
here *if and only if* you used kfree. By doing an unchecked kmem_cache_free
like this, you will immediately hit an Oops in the first lookup (see below).
Please, make sure you actually stress test this patchset with fstests
against both f2fs and ext4 before sending each new version.
Thanks,
[ 74.202044] F2FS-fs (loop0): Using encoding defined by superblock:
utf8-12.1.0 with flags 0x0
[ 74.206592] F2FS-fs (loop0): Found nat_bits in checkpoint
[ 74.221467] F2FS-fs (loop0): Mounted with checkpoint version = 3e684111
FSTYP -- f2fs
PLATFORM -- Linux/x86_64 sle15sp5 6.7.0-gf27274eae416 #8 SMP
PREEMPT_DYNAMIC Thu Mar 14 00:22:47 CET 2024
MKFS_OPTIONS -- -O encrypt /dev/loop1
MOUNT_OPTIONS -- -o acl,user_xattr /dev/loop1 /root/work/scratch
[ 75.038385] F2FS-fs (loop1): Found nat_bits in checkpoint
[ 75.054311] F2FS-fs (loop1): Mounted with checkpoint version = 6b9fbccb
[ 75.176328] F2FS-fs (loop0): Using encoding defined by superblock:
utf8-12.1.0 with flags 0x0
[ 75.179261] F2FS-fs (loop0): Found nat_bits in checkpoint
[ 75.194264] F2FS-fs (loop0): Mounted with checkpoint version = 3e684114
f2fs/001 1s ... [ 75.570867] run fstests f2fs/001 at 2024-03-14 00:24:33
[ 75.753604] BUG: unable to handle page fault for address: f14ad208
[ 75.754209] #PF: supervisor read access in kernel mode
[ 75.754647] #PF: error_code(0x) - not-present page
[ 75.755077] PGD 0 P4D 0
[ 75.755300] Oops: [#1] PREEMPT SMP NOPTI
[ 75.755683] CPU: 0 PID: 2740 Comm: xfs_io Not tainted 6.7.0-gf27274eae416 #8
[ 75.756266] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown
2/2/2022
[ 75.756911] RIP: 0010:kmem_cache_free+0x6a/0x320
[ 75.757309] Code: 80 48 01 d8 0f 82 b4 02 00 00 48 c7 c2 00 00 00 80 48 2b
15 f8 c2 18 01 48 01 d0 48 c1 e8 0c 48 c1 e0 06 48 03 05 d6 c2 18 01 <48> 8b 50
08 49 89 c6 f6 c2 01 0f 85 ea 01 00 00 0f 1f 44 00 00 49
[ 75.758834] RSP: 0018:a59bc231bb10 EFLAGS: 00010286
[ 75.759270] RAX: f14ad200 RBX: RCX:
[ 75.759860] RDX: 6204 RSI: RDI: 9dfc80043600
[ 75.760450] RBP: a59bc231bb30 R08: a59bc231b9a0 R09: 03fa
[ 75.761037] R10: 000fd024 R11: 0107 R12: 9dfc80043600
[ 75.761626] R13: 8404dc7a R14: R15: 9dfc8f1aa000
[ 75.762221] FS: 7f9601efb780() GS:9dfcfbc0()
knlGS:
[ 75.762888] CS: 0010 DS: ES: CR0: 80050033
[ 75.763372] CR2: f14ad208 CR3: 00011175 CR4: 00750ef0
[ 75.763962] PKRU: 5554
[ 75.764194] Call Trace:
[ 75.764435]
[ 75.764677] ? __die_body+0x1a/0x60
[ 75.764982] ? page_fault_oops+0x154/0x440
[ 75.765335] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.765760] ? search_module_extables+0x46/0x70
[ 75.766149] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.766548] ? fixup_exception+0x22/0x300
[ 75.766892] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.767292] ? exc_page_fault+0xa6/0x140
[ 75.767633] ? asm_exc_page_fault+0x22/0x30
[ 75.767995] ? f2fs_free_filename+0x2a/0x40
[ 75.768362] ? kmem_cache_free+0x6a/0x320
[ 75.768703] ? f2fs_free_filename+0x2a/0x40
[ 75.769061] f2fs_free_filename+0x2a/0x40
[ 75.769403] f2fs_lookup+0x19f/0x380
[ 75.769712] __lookup_slow+0x8b/0x130
[ 75.770034] walk_component+0xfc/0x170
[ 75.770353] path_lookupat+0x69/0x140
[ 75.770664] filename_lookup+0xe1/0x1c0
[ 75.770991] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.771393] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.771792] ? do_wp_page+0x3f6/0xbf0
[ 75.772109] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.772523] ? preempt_count_add+0x70/0xa0
[ 75.772902] ? vfs_statx+0x89/0x180
[ 75.773224] vfs_statx+0x89/0x180
[ 75.773530] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.773939] vfs_fstatat+0x80/0xa0
[ 75.774237] __do_sys_newfstatat+0x26/0x60
[ 75.774595] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.775021] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.775448] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.775878] ? do_user_addr_fault+0x563/0x7c0
[ 75.776273] ? srso_alias_return_thunk+0x5/0xfbef5
[ 75.776699] do_syscall_64+0x50/0x110
[ 75.777028] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 75.777479] RIP: 0033:0x7f9601b07aea
[ 75.93] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 90 90 90
90 90 90 90 90 90 90 90 90 90 90 90 90 41 89 ca b8 06 01 00 00 0f 05 <3d> 00 f0
ff ff 77 07 31 c0 c3 0f 1f 40 00 48 8b 15 01 23 0e 00 f7
[ 75.779391] RSP: 002b:7ffc160eaae8 EFLAGS: 0246 ORIG_RAX:
Re: [f2fs-dev] [PATCH v13 3/9] libfs: Introduce case-insensitive string comparison helper
Eugen Hristev writes:
> From: Gabriel Krisman Bertazi
>
> generic_ci_match can be used by case-insensitive filesystems to compare
> strings under lookup with dirents in a case-insensitive way. This
> function is currently reimplemented by each filesystem supporting
> casefolding, so this reduces code duplication in filesystem-specific
> code.
>
> Signed-off-by: Gabriel Krisman Bertazi
> [eugen.hris...@collabora.com: rework to first test the exact match]
> Signed-off-by: Eugen Hristev
> ---
> fs/libfs.c | 81 ++
> include/linux/fs.h | 4 +++
> 2 files changed, 85 insertions(+)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index c297953db948..c107c24f33b9 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1776,6 +1776,87 @@ static const struct dentry_operations
> generic_ci_dentry_ops = {
> .d_revalidate = fscrypt_d_revalidate,
> #endif
> };
> +
> +/**
> + * generic_ci_match() - Match a name (case-insensitively) with a dirent.
> + * This is a filesystem helper for comparison with directory entries.
> + * generic_ci_d_compare should be used in VFS' ->d_compare instead.
> + *
> + * @parent: Inode of the parent of the dirent under comparison
> + * @name: name under lookup.
> + * @folded_name: Optional pre-folded name under lookup
> + * @de_name: Dirent name.
> + * @de_name_len: dirent name length.
> + *
> + * Test whether a case-insensitive directory entry matches the filename
> + * being searched. If @folded_name is provided, it is used instead of
> + * recalculating the casefold of @name.
> + *
> + * Return: > 0 if the directory entry matches, 0 if it doesn't match, or
> + * < 0 on error.
> + */
> +int generic_ci_match(const struct inode *parent,
> + const struct qstr *name,
> + const struct qstr *folded_name,
> + const u8 *de_name, u32 de_name_len)
> +{
> + const struct super_block *sb = parent->i_sb;
> + const struct unicode_map *um = sb->s_encoding;
> + struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
> + struct qstr dirent = QSTR_INIT(de_name, de_name_len);
> + int res, match = 0;
> +
> + if (IS_ENCRYPTED(parent)) {
> + const struct fscrypt_str encrypted_name =
> + FSTR_INIT((u8 *) de_name, de_name_len);
> +
> + if (WARN_ON_ONCE(!fscrypt_has_encryption_key(parent)))
> + return -EINVAL;
> +
> + decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
> + if (!decrypted_name.name)
> + return -ENOMEM;
> + res = fscrypt_fname_disk_to_usr(parent, 0, 0, _name,
> + _name);
> + if (res < 0)
> + goto out;
> + dirent.name = decrypted_name.name;
> + dirent.len = decrypted_name.len;
> + }
> +
> + /*
> + * Attempt a case-sensitive match first. It is cheaper and
> + * should cover most lookups, including all the sane
> + * applications that expect a case-sensitive filesystem.
> + */
> + if (folded_name->name) {
> + if (dirent.len == folded_name->len &&
> + !memcmp(folded_name->name, dirent.name, dirent.len)) {
> + match = 1;
> + goto out;
> + }
> + res = utf8_strncasecmp_folded(um, folded_name, );
> + } else {
> + if (dirent.len == name->len &&
> + !memcmp(name->name, dirent.name, dirent.len) &&
> + (!sb_has_strict_encoding(sb) || !utf8_validate(um, name))) {
> + match = 1;
> + goto out;
> + }
> + res = utf8_strncasecmp(um, name, );
> + }
> +
> +out:
> + kfree(decrypted_name.name);
> + if (match) /* matched by direct comparison */
> + return 1;
> + else if (!res) /* matched by utf8 comparison */
> + return 1;
> + else if (res < 0) /* error on utf8 comparison */
> + return res;
> + return 0; /* no match */
> +}
I think I've made this comment before, but I'd prefer this to be written
in a much simpler way
if (res < 0)
return res;
return (match || !res);
Other than that, this looks good to me.
--
Gabriel Krisman Bertazi
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
