linux-nilfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2)
@ 2024-09-01 20:39 syzbot
  2024-09-02  8:41 ` Lizhi Xu
  2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
  0 siblings, 2 replies; 8+ messages in thread
From: syzbot @ 2024-09-01 20:39 UTC (permalink / raw)
  To: konishi.ryusuke, linux-kernel, linux-nilfs, syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    86987d84b968 Merge tag 'v6.11-rc5-client-fixes' of git://g..
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=10c2b835980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a0455552d0b27491
dashboard link: https://syzkaller.appspot.com/bug?extid=9bff4c7b992038a7409f
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=16fcc40d980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=143f1643980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/87692913ef45/disk-86987d84.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/a27da6973d7f/vmlinux-86987d84.xz
kernel image: https://storage.googleapis.com/syzbot-assets/2e28d02ce725/bzImage-86987d84.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/e3fe6fbe935d/mount_0.gz

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=17f5280d980000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=140d280d980000
console output: https://syzkaller.appspot.com/x/log.txt?x=100d280d980000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+9bff4c7b992038a7409f@syzkaller.appspotmail.com

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
CPU: 1 UID: 0 PID: 5220 Comm: syz-executor243 Not tainted 6.11.0-rc5-syzkaller-00057-g86987d84b968 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
RIP: 0010:nilfs_btree_get_nonroot_node fs/nilfs2/btree.c:418 [inline]
RIP: 0010:nilfs_btree_prepare_insert fs/nilfs2/btree.c:1091 [inline]
RIP: 0010:nilfs_btree_insert+0x6d3/0x1c10 fs/nilfs2/btree.c:1252
Code: 8d 9f 80 00 00 00 48 89 d8 48 c1 e8 03 42 80 3c 28 00 74 08 48 89 df e8 ab 89 8a fe 48 8b 1b 48 83 c3 28 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 8e 89 8a fe 4c 89 64 24 28 48 8b
RSP: 0018:ffffc9000352f4e0 EFLAGS: 00010206
RAX: 0000000000000005 RBX: 0000000000000028 RCX: 1ffff1100f66a0ee
RDX: ffff88807d6f9e00 RSI: 0000000000000002 RDI: 0000000000000001
RBP: ffffc9000352f670 R08: ffffffff836d1ea6 R09: 1ffff1100469939a
R10: dffffc0000000000 R11: ffffed100469939b R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000002 R15: ffff88807fbd9680
FS:  0000555573e9d380(0000) GS:ffff8880b9300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc785a110f8 CR3: 000000007ea40000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 nilfs_bmap_do_insert fs/nilfs2/bmap.c:129 [inline]
 nilfs_bmap_insert+0x25e/0x3c0 fs/nilfs2/bmap.c:155
 nilfs_get_block+0x428/0x8e0 fs/nilfs2/inode.c:101
 __block_write_begin_int+0x50c/0x1a70 fs/buffer.c:2125
 __block_write_begin fs/buffer.c:2174 [inline]
 block_write_begin+0x9b/0x1e0 fs/buffer.c:2235
 nilfs_write_begin+0xa0/0x110 fs/nilfs2/inode.c:262
 generic_perform_write+0x399/0x840 mm/filemap.c:4019
 generic_file_write_iter+0xaf/0x310 mm/filemap.c:4147
 new_sync_write fs/read_write.c:497 [inline]
 vfs_write+0xa72/0xc90 fs/read_write.c:590
 ksys_write+0x1a0/0x2c0 fs/read_write.c:643
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fc785999a99
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff8363c328 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fc785999a99
RDX: 0000000000004000 RSI: 0000000020000000 RDI: 0000000000000004
RBP: 00007fc785a0d5f0 R08: 0000555573e9e4c0 R09: 0000555573e9e4c0
R10: 0000000000000a83 R11: 0000000000000246 R12: 00007fff8363c350
R13: 00007fff8363c578 R14: 431bde82d7b634db R15: 00007fc7859e203b
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:nilfs_btree_get_nonroot_node fs/nilfs2/btree.c:418 [inline]
RIP: 0010:nilfs_btree_prepare_insert fs/nilfs2/btree.c:1091 [inline]
RIP: 0010:nilfs_btree_insert+0x6d3/0x1c10 fs/nilfs2/btree.c:1252
Code: 8d 9f 80 00 00 00 48 89 d8 48 c1 e8 03 42 80 3c 28 00 74 08 48 89 df e8 ab 89 8a fe 48 8b 1b 48 83 c3 28 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 8e 89 8a fe 4c 89 64 24 28 48 8b
RSP: 0018:ffffc9000352f4e0 EFLAGS: 00010206
RAX: 0000000000000005 RBX: 0000000000000028 RCX: 1ffff1100f66a0ee
RDX: ffff88807d6f9e00 RSI: 0000000000000002 RDI: 0000000000000001
RBP: ffffc9000352f670 R08: ffffffff836d1ea6 R09: 1ffff1100469939a
R10: dffffc0000000000 R11: ffffed100469939b R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000002 R15: ffff88807fbd9680
FS:  0000555573e9d380(0000) GS:ffff8880b9200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005582b0414000 CR3: 000000007ea40000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
   0:	8d 9f 80 00 00 00    	lea    0x80(%rdi),%ebx
   6:	48 89 d8             	mov    %rbx,%rax
   9:	48 c1 e8 03          	shr    $0x3,%rax
   d:	42 80 3c 28 00       	cmpb   $0x0,(%rax,%r13,1)
  12:	74 08                	je     0x1c
  14:	48 89 df             	mov    %rbx,%rdi
  17:	e8 ab 89 8a fe       	call   0xfe8a89c7
  1c:	48 8b 1b             	mov    (%rbx),%rbx
  1f:	48 83 c3 28          	add    $0x28,%rbx
  23:	48 89 d8             	mov    %rbx,%rax
  26:	48 c1 e8 03          	shr    $0x3,%rax
* 2a:	42 80 3c 28 00       	cmpb   $0x0,(%rax,%r13,1) <-- trapping instruction
  2f:	74 08                	je     0x39
  31:	48 89 df             	mov    %rbx,%rdi
  34:	e8 8e 89 8a fe       	call   0xfe8a89c7
  39:	4c 89 64 24 28       	mov    %r12,0x28(%rsp)
  3e:	48                   	rex.W
  3f:	8b                   	.byte 0x8b


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2)
  2024-09-01 20:39 [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2) syzbot
@ 2024-09-02  8:41 ` Lizhi Xu
  2024-09-02 10:08   ` syzbot
  2024-09-02 19:40   ` Ryusuke Konishi
  2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
  1 sibling, 2 replies; 8+ messages in thread
From: Lizhi Xu @ 2024-09-02  8:41 UTC (permalink / raw)
  To: syzbot+9bff4c7b992038a7409f
  Cc: konishi.ryusuke, linux-kernel, linux-nilfs, syzkaller-bugs

In nilfs_btree_do_lookup, if the number of children in the btree root node is 0,
path[x].bp_bh will not be initialized by __nilfs_btree_get_block,
which will result in uaf when executing nilfs-btree_get_nonroot_node
in nilfs_btree_prepare_insert.

In nilfs_bmap_do_insert will run bop_check_insert, so implement
bop_check_insert and determine the number of children in the btree root
node within it. If it is 0, return a negative value to avoid calling
bop_intsert.

#syz test

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index 862bdf23120e..d7fa4d914638 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -1231,6 +1231,17 @@ static void nilfs_btree_commit_insert(struct nilfs_bmap *btree,
 		nilfs_bmap_set_dirty(btree);
 }
 
+static int nilfs_btree_check_insert(const struct nilfs_bmap *btree, __u64 key)
+{
+	struct nilfs_btree_node *node;
+	int level;
+
+	node = nilfs_btree_get_root(btree);
+	level = nilfs_btree_node_get_level(node);
+	return (level < NILFS_BTREE_LEVEL_NODE_MIN ||
+		nilfs_btree_node_get_nchildren(node) <= 0) ? -ENOENT : 0;
+}
+
 static int nilfs_btree_insert(struct nilfs_bmap *btree, __u64 key, __u64 ptr)
 {
 	struct nilfs_btree_path *path;
@@ -2385,7 +2396,7 @@ static const struct nilfs_bmap_operations nilfs_btree_ops = {
 	.bop_seek_key		=	nilfs_btree_seek_key,
 	.bop_last_key		=	nilfs_btree_last_key,
 
-	.bop_check_insert	=	NULL,
+	.bop_check_insert	=	nilfs_btree_check_insert,
 	.bop_check_delete	=	nilfs_btree_check_delete,
 	.bop_gather_data	=	nilfs_btree_gather_data,
 };

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2)
  2024-09-02  8:41 ` Lizhi Xu
@ 2024-09-02 10:08   ` syzbot
  2024-09-02 19:40   ` Ryusuke Konishi
  1 sibling, 0 replies; 8+ messages in thread
From: syzbot @ 2024-09-02 10:08 UTC (permalink / raw)
  To: konishi.ryusuke, linux-kernel, linux-nilfs, lizhi.xu,
	syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+9bff4c7b992038a7409f@syzkaller.appspotmail.com
Tested-by: syzbot+9bff4c7b992038a7409f@syzkaller.appspotmail.com

Tested on:

commit:         67784a74 Merge tag 'ata-6.11-rc7' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1752cf2b980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=94f4a91e4d2a2d49
dashboard link: https://syzkaller.appspot.com/bug?extid=9bff4c7b992038a7409f
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1012cf2b980000

Note: testing is done by a robot and is best-effort only.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2)
  2024-09-02  8:41 ` Lizhi Xu
  2024-09-02 10:08   ` syzbot
@ 2024-09-02 19:40   ` Ryusuke Konishi
  1 sibling, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2024-09-02 19:40 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: syzbot+9bff4c7b992038a7409f, linux-kernel, linux-nilfs,
	syzkaller-bugs

On Mon, Sep 2, 2024 at 5:41 PM Lizhi Xu wrote:
>
> In nilfs_btree_do_lookup, if the number of children in the btree root node is 0,
> path[x].bp_bh will not be initialized by __nilfs_btree_get_block,
> which will result in uaf when executing nilfs-btree_get_nonroot_node
> in nilfs_btree_prepare_insert.
>
> In nilfs_bmap_do_insert will run bop_check_insert, so implement
> bop_check_insert and determine the number of children in the btree root
> node within it. If it is 0, return a negative value to avoid calling
> bop_intsert.

Thank you Lizhi for helping us solve this problem.

However, your proposed patch has a few issues (more on that later),
and this anomaly detection should be done when the root node is read,
not just before insertion.  So, instead of adding bop_check_insert, I
will modify the existing sanity check routine
nilfs_btree_root_broken() by adding the following failure condition:

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index 862bdf23120e..d390b8ba00d4 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -381,7 +381,8 @@ static int nilfs_btree_root_broken(const struct
nilfs_btree_node *node,
        if (unlikely(level < NILFS_BTREE_LEVEL_NODE_MIN ||
                     level >= NILFS_BTREE_LEVEL_MAX ||
                     nchildren < 0 ||
-                    nchildren > NILFS_BTREE_ROOT_NCHILDREN_MAX)) {
+                    nchildren > NILFS_BTREE_ROOT_NCHILDREN_MAX ||
+                    (nchildren == 0 && level > NILFS_BTREE_LEVEL_NODE_MIN))) {
                nilfs_crit(inode->i_sb,
                           "bad btree root (ino=%lu): level = %d,
flags = 0x%x, nchildren = %d",
                           inode->i_ino, level, flags, nchildren);

---
Going back to your proposed patch, there are two problems:

(1) nilfs_btree_node_get_nchildren(node) == 0 is a normal state in
itself, so it is not OK to return this as an error.

(2) Even if we use bop_check_insert to perform a sanity check, the
error code that should be returned is -EINVAL.
-ENOENT is an internal error code, but if bop_check_insert() returns
it, it may be wrongly exposed to userspace (i.e. system
calls).  If it is -EINVAL, it will be treated as metadata corruption
at the bmap layer with nilfs_bmap_convert_error(), and the error code
will be converted for return to userspace.

So this time I'll be fixing and testing it myself, and will mention
your help in the patch commit log.

Thanks,
Ryusuke Konishi

>
> #syz test
>
> diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
> index 862bdf23120e..d7fa4d914638 100644
> --- a/fs/nilfs2/btree.c
> +++ b/fs/nilfs2/btree.c
> @@ -1231,6 +1231,17 @@ static void nilfs_btree_commit_insert(struct nilfs_bmap *btree,
>                 nilfs_bmap_set_dirty(btree);
>  }
>
> +static int nilfs_btree_check_insert(const struct nilfs_bmap *btree, __u64 key)
> +{
> +       struct nilfs_btree_node *node;
> +       int level;
> +
> +       node = nilfs_btree_get_root(btree);
> +       level = nilfs_btree_node_get_level(node);
> +       return (level < NILFS_BTREE_LEVEL_NODE_MIN ||
> +               nilfs_btree_node_get_nchildren(node) <= 0) ? -ENOENT : 0;
> +}
> +
>  static int nilfs_btree_insert(struct nilfs_bmap *btree, __u64 key, __u64 ptr)
>  {
>         struct nilfs_btree_path *path;
> @@ -2385,7 +2396,7 @@ static const struct nilfs_bmap_operations nilfs_btree_ops = {
>         .bop_seek_key           =       nilfs_btree_seek_key,
>         .bop_last_key           =       nilfs_btree_last_key,
>
> -       .bop_check_insert       =       NULL,
> +       .bop_check_insert       =       nilfs_btree_check_insert,
>         .bop_check_delete       =       nilfs_btree_check_delete,
>         .bop_gather_data        =       nilfs_btree_gather_data,
>  };

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes
  2024-09-01 20:39 [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2) syzbot
  2024-09-02  8:41 ` Lizhi Xu
@ 2024-09-04  8:13 ` Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 1/3] nilfs2: fix potential null-ptr-deref in nilfs_btree_insert() Ryusuke Konishi
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2024-09-04  8:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-nilfs, syzbot, syzkaller-bugs, LKML

Hi Andrew,

please add this series to the queue for the next cycle.

This addresses three potential issues with empty b-tree nodes that can
occur with corrupted filesystem images, including one recently
discovered by syzbot.

Thanks,
Ryusuke Konishi

Ryusuke Konishi (3):
  nilfs2: fix potential null-ptr-deref in nilfs_btree_insert()
  nilfs2: determine empty node blocks as corrupted
  nilfs2: fix potential oob read in nilfs_btree_check_delete()

 fs/nilfs2/btree.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] nilfs2: fix potential null-ptr-deref in nilfs_btree_insert()
  2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
@ 2024-09-04  8:13   ` Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 2/3] nilfs2: determine empty node blocks as corrupted Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 3/3] nilfs2: fix potential oob read in nilfs_btree_check_delete() Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2024-09-04  8:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-nilfs, syzbot, syzkaller-bugs, LKML

If a b-tree is broken on the device, and the b-tree height is greater
than 2 (the level of the root node is greater than 1) even if the
number of child nodes of the b-tree root is 0, a NULL pointer
dereference occurs in nilfs_btree_prepare_insert(), which is called
from nilfs_btree_insert().

This is because, when the number of child nodes of the b-tree root is
0, nilfs_btree_do_lookup() does not set the block buffer head in any
of path[x].bp_bh, leaving it as the initial value of NULL, but if the
level of the b-tree root node is greater than 1,
nilfs_btree_get_nonroot_node(), which accesses the buffer memory of
path[x].bp_bh, is called.

Fix this issue by adding a check to nilfs_btree_root_broken(), which
performs sanity checks when reading the root node from the device, to
detect this inconsistency.

Thanks to Lizhi Xu for trying to solve the bug and clarifying the
cause early on.

Link: https://lkml.kernel.org/r/20240902084101.138971-1-lizhi.xu@windriver.com
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Reported-by: syzbot+9bff4c7b992038a7409f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9bff4c7b992038a7409f
Fixes: 17c76b0104e4 ("nilfs2: B-tree based block mapping")
---
 fs/nilfs2/btree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index 862bdf23120e..d390b8ba00d4 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -381,7 +381,8 @@ static int nilfs_btree_root_broken(const struct nilfs_btree_node *node,
 	if (unlikely(level < NILFS_BTREE_LEVEL_NODE_MIN ||
 		     level >= NILFS_BTREE_LEVEL_MAX ||
 		     nchildren < 0 ||
-		     nchildren > NILFS_BTREE_ROOT_NCHILDREN_MAX)) {
+		     nchildren > NILFS_BTREE_ROOT_NCHILDREN_MAX ||
+		     (nchildren == 0 && level > NILFS_BTREE_LEVEL_NODE_MIN))) {
 		nilfs_crit(inode->i_sb,
 			   "bad btree root (ino=%lu): level = %d, flags = 0x%x, nchildren = %d",
 			   inode->i_ino, level, flags, nchildren);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] nilfs2: determine empty node blocks as corrupted
  2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 1/3] nilfs2: fix potential null-ptr-deref in nilfs_btree_insert() Ryusuke Konishi
@ 2024-09-04  8:13   ` Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 3/3] nilfs2: fix potential oob read in nilfs_btree_check_delete() Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2024-09-04  8:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-nilfs, syzbot, syzkaller-bugs, LKML

Due to the nature of b-trees, nilfs2 itself and admin tools such as
mkfs.nilfs2 will never create an intermediate b-tree node block with 0
child nodes, nor will they delete (key, pointer)-entries that would
result in such a state.  However, it is possible that a b-tree node
block is corrupted on the backing device and is read with 0 child
nodes.

Because operation is not guaranteed if the number of child nodes is 0
for intermediate node blocks other than the root node, modify
nilfs_btree_node_broken(), which performs sanity checks when reading a
b-tree node block, so that such cases will be judged as metadata
corruption.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Fixes: 17c76b0104e4 ("nilfs2: B-tree based block mapping")
---
 fs/nilfs2/btree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index d390b8ba00d4..dedd3c480842 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -350,7 +350,7 @@ static int nilfs_btree_node_broken(const struct nilfs_btree_node *node,
 	if (unlikely(level < NILFS_BTREE_LEVEL_NODE_MIN ||
 		     level >= NILFS_BTREE_LEVEL_MAX ||
 		     (flags & NILFS_BTREE_NODE_ROOT) ||
-		     nchildren < 0 ||
+		     nchildren <= 0 ||
 		     nchildren > NILFS_BTREE_NODE_NCHILDREN_MAX(size))) {
 		nilfs_crit(inode->i_sb,
 			   "bad btree node (ino=%lu, blocknr=%llu): level = %d, flags = 0x%x, nchildren = %d",
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] nilfs2: fix potential oob read in nilfs_btree_check_delete()
  2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 1/3] nilfs2: fix potential null-ptr-deref in nilfs_btree_insert() Ryusuke Konishi
  2024-09-04  8:13   ` [PATCH 2/3] nilfs2: determine empty node blocks as corrupted Ryusuke Konishi
@ 2024-09-04  8:13   ` Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2024-09-04  8:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-nilfs, syzbot, syzkaller-bugs, LKML

The function nilfs_btree_check_delete(), which checks whether
degeneration to direct mapping occurs before deleting a b-tree entry,
causes memory access outside the block buffer when retrieving the
maximum key if the root node has no entries.

This does not usually happen because b-tree mappings with 0 child
nodes are never created by mkfs.nilfs2 or nilfs2 itself.  However, it
can happen if the b-tree root node read from a device is configured
that way, so fix this potential issue by adding a check for that case.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Fixes: 17c76b0104e4 ("nilfs2: B-tree based block mapping")
---
 fs/nilfs2/btree.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index dedd3c480842..ef5061bb56da 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -1659,13 +1659,16 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
 	int nchildren, ret;
 
 	root = nilfs_btree_get_root(btree);
+	nchildren = nilfs_btree_node_get_nchildren(root);
+	if (unlikely(nchildren == 0))
+		return 0;
+
 	switch (nilfs_btree_height(btree)) {
 	case 2:
 		bh = NULL;
 		node = root;
 		break;
 	case 3:
-		nchildren = nilfs_btree_node_get_nchildren(root);
 		if (nchildren > 1)
 			return 0;
 		ptr = nilfs_btree_node_get_ptr(root, nchildren - 1,
@@ -1674,12 +1677,12 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
 		if (ret < 0)
 			return ret;
 		node = (struct nilfs_btree_node *)bh->b_data;
+		nchildren = nilfs_btree_node_get_nchildren(node);
 		break;
 	default:
 		return 0;
 	}
 
-	nchildren = nilfs_btree_node_get_nchildren(node);
 	maxkey = nilfs_btree_node_get_key(node, nchildren - 1);
 	nextmaxkey = (nchildren > 1) ?
 		nilfs_btree_node_get_key(node, nchildren - 2) : 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-09-04  8:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-01 20:39 [syzbot] [nilfs?] general protection fault in nilfs_btree_insert (2) syzbot
2024-09-02  8:41 ` Lizhi Xu
2024-09-02 10:08   ` syzbot
2024-09-02 19:40   ` Ryusuke Konishi
2024-09-04  8:13 ` [PATCH 0/3] nilfs2: fix potential issues with empty b-tree nodes Ryusuke Konishi
2024-09-04  8:13   ` [PATCH 1/3] nilfs2: fix potential null-ptr-deref in nilfs_btree_insert() Ryusuke Konishi
2024-09-04  8:13   ` [PATCH 2/3] nilfs2: determine empty node blocks as corrupted Ryusuke Konishi
2024-09-04  8:13   ` [PATCH 3/3] nilfs2: fix potential oob read in nilfs_btree_check_delete() Ryusuke Konishi

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).