[PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Deepanshu Kartikey]

nilfs_ioctl_mark_blocks_dirty() calls nilfs_bmap_lookup_at_level() to
get the current block number of each block descriptor. When the lookup
returns -ENOENT, meaning the block does not exist, it sets bd_blocknr
to 0 and continues processing.

However, if bd_oblocknr is also 0, the subsequent check:

  if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
          continue;

will not skip the block, and nilfs_bmap_mark() will be called on a
non-existent block. This causes nilfs_btree_do_lookup() to return
-ENOENT, triggering the WARN_ON(ret == -ENOENT).

Fix this by adding a continue statement after setting bd_blocknr to 0
when the lookup returns -ENOENT, so that dead blocks are always skipped
regardless of the value of bd_oblocknr.

Fixes: 7942b919f732 ("nilfs2: ioctl operations")
Reported-by: syzbot+98a040252119df0506f8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=98a040252119df0506f8
Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
---
 fs/nilfs2/ioctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index e17b8da66491..1615a314f557 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -745,6 +745,7 @@ static int nilfs_ioctl_mark_blocks_dirty(struct the_nilfs *nilfs,
 			if (ret != -ENOENT)
 				return ret;
 			bdescs[i].bd_blocknr = 0;
+			continue;
 		}
 		if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
 			/* skip dead block */
-- 
2.43.0

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Ryusuke Konishi]

Thank you, Deepanshu.

On Thu, Mar 19, 2026 at 6:19 PM Deepanshu Kartikey wrote:
>
> nilfs_ioctl_mark_blocks_dirty() calls nilfs_bmap_lookup_at_level() to
> get the current block number of each block descriptor. When the lookup
> returns -ENOENT, meaning the block does not exist, it sets bd_blocknr
> to 0 and continues processing.
>
> However, if bd_oblocknr is also 0, the subsequent check:
>
>   if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
>           continue;
>
> will not skip the block, and nilfs_bmap_mark() will be called on a
> non-existent block. This causes nilfs_btree_do_lookup() to return
> -ENOENT, triggering the WARN_ON(ret == -ENOENT).
>
> Fix this by adding a continue statement after setting bd_blocknr to 0
> when the lookup returns -ENOENT, so that dead blocks are always skipped
> regardless of the value of bd_oblocknr.
>
> Fixes: 7942b919f732 ("nilfs2: ioctl operations")
> Reported-by: syzbot+98a040252119df0506f8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=98a040252119df0506f8
> Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
> ---
>  fs/nilfs2/ioctl.c | 1 +
>  1 file changed, 1 insertion(+)

Since this implementation interacts with userland GC, I will check
whether this is a simple missing 'continue' statement or if it was
intentional.
If it is as you pointed out, I will pick it up and send it upstream.

Thanks,
Ryusuke Konishi

>
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index e17b8da66491..1615a314f557 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -745,6 +745,7 @@ static int nilfs_ioctl_mark_blocks_dirty(struct the_nilfs *nilfs,
>                         if (ret != -ENOENT)
>                                 return ret;
>                         bdescs[i].bd_blocknr = 0;
> +                       continue;
>                 }
>                 if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
>                         /* skip dead block */
> --
> 2.43.0
>

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Junjie Cao]

Hi Ryusuke,

On Sat, 21 Mar 2026 at 02:32:05 +0900, Ryusuke Konishi wrote:
> Since this implementation interacts with userland GC, I will check
> whether this is a simple missing 'continue' statement or if it was
> intentional.

Both downstream paths have asserted on (ret == -ENOENT) since the
original commit 7942b919f732 -- initially as BUG_ON, later softened
to WARN_ON by 1f5abe7e7dbc.  If -ENOENT were meant to reach those
paths, asserting on it would be contradictory.

The original code appears to rely on the dead-block check
(bd_blocknr != bd_oblocknr) to implicitly skip the -ENOENT case,
which breaks when bd_oblocknr is also 0.

This same fix also resolves a related syzbot report that hits the
same root cause through the level-0 path (nilfs_mdt_get_block)
rather than nilfs_bmap_mark.  I applied the patch on top of
current master (bbeb83d3182a) and tested it locally against that
report's C reproducer in QEMU -- the warning no longer triggers.

  https://syzkaller.appspot.com/bug?extid=466a45fcfb0562f5b9a0

For that related report, when the patch is picked up:

  Reported-by: syzbot+466a45fcfb0562f5b9a0@syzkaller.appspotmail.com
  Closes: https://syzkaller.appspot.com/bug?extid=466a45fcfb0562f5b9a0
  Tested-by: Junjie Cao <junjie.cao@linux.dev>

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Deepanshu Kartikey]

On Fri, Mar 20, 2026 at 11:02 PM Ryusuke Konishi
<konishi.ryusuke@gmail.com> wrote:
>
> Thank you, Deepanshu.
>
> On Thu, Mar 19, 2026 at 6:19 PM Deepanshu Kartikey wrote:
> >
> > nilfs_ioctl_mark_blocks_dirty() calls nilfs_bmap_lookup_at_level() to
> > get the current block number of each block descriptor. When the lookup
> > returns -ENOENT, meaning the block does not exist, it sets bd_blocknr
> > to 0 and continues processing.
> >
> > However, if bd_oblocknr is also 0, the subsequent check:
> >
> >   if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
> >           continue;
> >
> > will not skip the block, and nilfs_bmap_mark() will be called on a
> > non-existent block. This causes nilfs_btree_do_lookup() to return
> > -ENOENT, triggering the WARN_ON(ret == -ENOENT).
> >
> > Fix this by adding a continue statement after setting bd_blocknr to 0
> > when the lookup returns -ENOENT, so that dead blocks are always skipped
> > regardless of the value of bd_oblocknr.
> >
> > Fixes: 7942b919f732 ("nilfs2: ioctl operations")
> > Reported-by: syzbot+98a040252119df0506f8@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=98a040252119df0506f8
> > Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
> > ---
> >  fs/nilfs2/ioctl.c | 1 +
> >  1 file changed, 1 insertion(+)
>
> Since this implementation interacts with userland GC, I will check
> whether this is a simple missing 'continue' statement or if it was
> intentional.
> If it is as you pointed out, I will pick it up and send it upstream.
>
> Thanks,
> Ryusuke Konishi
>

gentle ping on this patch. Would like to know the status.
Let me know anything is required from my side

Thanks

Deepanshu

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Ryusuke Konishi]

Hi Deepanshu,

On Mon, Mar 30, 2026 at 6:40 PM Deepanshu Kartikey wrote:
>
> On Fri, Mar 20, 2026 at 11:02 PM Ryusuke Konishi
> <konishi.ryusuke@gmail.com> wrote:
> >
> > Thank you, Deepanshu.
> >
> > On Thu, Mar 19, 2026 at 6:19 PM Deepanshu Kartikey wrote:
> > >
> > > nilfs_ioctl_mark_blocks_dirty() calls nilfs_bmap_lookup_at_level() to
> > > get the current block number of each block descriptor. When the lookup
> > > returns -ENOENT, meaning the block does not exist, it sets bd_blocknr
> > > to 0 and continues processing.
> > >
> > > However, if bd_oblocknr is also 0, the subsequent check:
> > >
> > >   if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
> > >           continue;
> > >
> > > will not skip the block, and nilfs_bmap_mark() will be called on a
> > > non-existent block. This causes nilfs_btree_do_lookup() to return
> > > -ENOENT, triggering the WARN_ON(ret == -ENOENT).
> > >
> > > Fix this by adding a continue statement after setting bd_blocknr to 0
> > > when the lookup returns -ENOENT, so that dead blocks are always skipped
> > > regardless of the value of bd_oblocknr.
> > >
> > > Fixes: 7942b919f732 ("nilfs2: ioctl operations")
> > > Reported-by: syzbot+98a040252119df0506f8@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=98a040252119df0506f8
> > > Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
> > > ---
> > >  fs/nilfs2/ioctl.c | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Since this implementation interacts with userland GC, I will check
> > whether this is a simple missing 'continue' statement or if it was
> > intentional.
> > If it is as you pointed out, I will pick it up and send it upstream.
> >
> > Thanks,
> > Ryusuke Konishi
> >
>
> gentle ping on this patch. Would like to know the status.
> Let me know anything is required from my side
>
> Thanks
>
> Deepanshu

Please wait a little longer.

I have finished reviewing your other patch and am currently testing it.

I'm sorry for the delay.  I was unable to work last week due to a
family bereavement and various administrative procedures that
followed.

Regards,
Ryusuke Konishi

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Ryusuke Konishi]

(added Junjie Cao to CC)

Hi Deepanshu,

On Mon, Mar 30, 2026 at 7:00 PM Ryusuke Konishi wrote:
>
> Hi Deepanshu,
>
> On Mon, Mar 30, 2026 at 6:40 PM Deepanshu Kartikey wrote:
> >
> > On Fri, Mar 20, 2026 at 11:02 PM Ryusuke Konishi
> > <konishi.ryusuke@gmail.com> wrote:
> > >
> > > Thank you, Deepanshu.
> > >
> > > On Thu, Mar 19, 2026 at 6:19 PM Deepanshu Kartikey wrote:
> > > >
> > > > nilfs_ioctl_mark_blocks_dirty() calls nilfs_bmap_lookup_at_level() to
> > > > get the current block number of each block descriptor. When the lookup
> > > > returns -ENOENT, meaning the block does not exist, it sets bd_blocknr
> > > > to 0 and continues processing.
> > > >
> > > > However, if bd_oblocknr is also 0, the subsequent check:
> > > >
> > > >   if (bdescs[i].bd_blocknr != bdescs[i].bd_oblocknr)
> > > >           continue;
> > > >
> > > > will not skip the block, and nilfs_bmap_mark() will be called on a
> > > > non-existent block. This causes nilfs_btree_do_lookup() to return
> > > > -ENOENT, triggering the WARN_ON(ret == -ENOENT).
> > > >
> > > > Fix this by adding a continue statement after setting bd_blocknr to 0
> > > > when the lookup returns -ENOENT, so that dead blocks are always skipped
> > > > regardless of the value of bd_oblocknr.
> > > >
> > > > Fixes: 7942b919f732 ("nilfs2: ioctl operations")
> > > > Reported-by: syzbot+98a040252119df0506f8@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=98a040252119df0506f8
> > > > Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
> > > > ---
> > > >  fs/nilfs2/ioctl.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > >
> > > Since this implementation interacts with userland GC, I will check
> > > whether this is a simple missing 'continue' statement or if it was
> > > intentional.
> > > If it is as you pointed out, I will pick it up and send it upstream.
> > >
> > > Thanks,
> > > Ryusuke Konishi
> > >
> >
> > gentle ping on this patch. Would like to know the status.
> > Let me know anything is required from my side
> >
> > Thanks
> >
> > Deepanshu
>
> Please wait a little longer.
>
> I have finished reviewing your other patch and am currently testing it.
>
> I'm sorry for the delay.  I was unable to work last week due to a
> family bereavement and various administrative procedures that
> followed.
>
> Regards,
> Ryusuke Konishi

I checked this and found that this fallthrough was intentional, aiming
to detect and skip a dead block by the subsequent comparison with
bd_oblocknr.

The problem is that it does not reject cases where bd_oblocknr takes
an unexpected value of 0.

This bd_oblocknr parameter stores the location where the userland GC
library found the target data block or the target intermediate block
of the DAT file.
As long as it is valid, it can never be block 0, which typically
stores the primary superblock and others.

However, due to the missing check for that anomalous value, the
corrupted ioctl request pattern generated by syzbot triggers the
assertion failure you pointed out.
I think a check like the following should be inserted at the beginning
of each iteration of the loop:

if (unlikely(!bdescs[i].bd_oblocknr))
    return -EINVAL;

Could you please restructure the patch in that direction?

Thanks,
Ryusuke Konishi

Re: [PATCH] nilfs2: fix missing continue after -ENOENT in nilfs_ioctl_mark_blocks_dirty()

[Deepanshu Kartikey]

On Mon, Mar 30, 2026 at 11:39 PM Ryusuke Konishi
<konishi.ryusuke@gmail.com> wrote:
>
> (added Junjie Cao to CC)
>
> Hi Deepanshu,
>
>
> I checked this and found that this fallthrough was intentional, aiming
> to detect and skip a dead block by the subsequent comparison with
> bd_oblocknr.
>
> The problem is that it does not reject cases where bd_oblocknr takes
> an unexpected value of 0.
>
> This bd_oblocknr parameter stores the location where the userland GC
> library found the target data block or the target intermediate block
> of the DAT file.
> As long as it is valid, it can never be block 0, which typically
> stores the primary superblock and others.
>
> However, due to the missing check for that anomalous value, the
> corrupted ioctl request pattern generated by syzbot triggers the
> assertion failure you pointed out.
> I think a check like the following should be inserted at the beginning
> of each iteration of the loop:
>
> if (unlikely(!bdescs[i].bd_oblocknr))
>     return -EINVAL;
>
> Could you please restructure the patch in that direction?
>
> Thanks,
> Ryusuke Konishi

Hi Ryusuke,

Thanks for the clarification. I have sent patch v2

Thanks

Deepanshu