linux-kernel - Re: [PATCH v3] nilfs2: Fix potential block overflow that cause system hang

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKFNMomckE-AKyZE5jJvKPn9-fWSEncqMPX=PP+cr3j6y==yDg@mail.gmail.com>
Date: Thu, 18 Dec 2025 20:32:49 +0900
From: Ryusuke Konishi <konishi.ryusuke@...il.com>
To: Edward Adam Davis <eadavis@...com>
Cc: slava@...eyko.com, linux-nilfs@...r.kernel.org, 
	syzkaller-bugs@...glegroups.com, axboe@...nel.dk, kristian@...usen.dk, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] nilfs2: Fix potential block overflow that cause system hang

On Thu, Dec 18, 2025 at 1:22 PM Edward Adam Davis wrote:
>
> When a user executes the FITRIM command, an underflow can occur when
> calculating nblocks if end_block is too small. Since nblocks is of
> type sector_t, which is u64, a negative nblocks value will become a
> very large positive integer. This ultimately leads to the block layer
> function __blkdev_issue_discard() taking an excessively long time to
> process the bio chain, and the ns_segctor_sem lock remains held for a
> long period. This prevents other tasks from acquiring the ns_segctor_sem
> lock, resulting in the hang reported by syzbot in [1].
>
> If the ending block is too small, for example, smaller than first data
> block, this poses a risk of corrupting the filesystem's superblock.
> Here, I check if the segment's ending block number is 0 to determine
> if the previously calculated ending block is too small.
>
> Although the start and len values in the user input range are too small,
> a conservative strategy is adopted here to safely ignore them, which is
> equivalent to a no-op; it will not perform any trimming and will not
> throw an error.
>
> [1]
> task:segctord state:D stack:28968 pid:6093 tgid:6093  ppid:2 task_flags:0x200040 flags:0x00080000
> Call Trace:
>  rwbase_write_lock+0x3dd/0x750 kernel/locking/rwbase_rt.c:272
>  nilfs_transaction_lock+0x253/0x4c0 fs/nilfs2/segment.c:357
>  nilfs_segctor_thread_construct fs/nilfs2/segment.c:2569 [inline]
>  nilfs_segctor_thread+0x6ec/0xe00 fs/nilfs2/segment.c:2684
>
> Fixes: 82e11e857be3 ("nilfs2: add nilfs_sufile_trim_fs to trim clean segs")
> Reported-by: syzbot+7eedce5eb281acd832f0@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7eedce5eb281acd832f0
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
> v2 -> v3: change to segment end check and update comments
> v1 -> v2: continue do discard and comments
>
>  fs/nilfs2/sufile.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
> index 83f93337c01b..fa612d5ec726 100644
> --- a/fs/nilfs2/sufile.c
> +++ b/fs/nilfs2/sufile.c
> @@ -1095,6 +1095,8 @@ int nilfs_sufile_trim_fs(struct inode *sufile, struct fstrim_range *range)
>
>         segnum = nilfs_get_segnum_of_block(nilfs, start_block);
>         segnum_end = nilfs_get_segnum_of_block(nilfs, end_block);
> +       if (!segnum_end)
> +               return 0;
>
>         down_read(&NILFS_MDT(sufile)->mi_sem);
>
> --
> 2.43.0

Hi Edward,

Thanks for submitting the patches.

However, I see two issues with the v3 patch:

First, this patch results in ignoring discard requests that are
limited to the region within segment number 0.
This is not the desired behavior.

Also, the final processing step that sets the actual discarded byte
size to range->len gets skipped.
When exiting normally, the code needs to goto a (new) label just
before the following assignment:

range->len = ndiscarded << nilfs->ns_blocksize_bits;

The root cause lies in the logic that clips the last free extent to
fit within the range specified by the ioctl.
As you noticed in your v2 patch, the issue is that end_block (the end
of the clipping region) is located before start (the start position of
the free extent), which causes an underflow in the following nblocks
calculation:

if (start + nblocks > end_block + 1)
        nblocks = end_block - start + 1;

The reason this happens is that the beginning of segment 0 is
truncated to reserve space for the primary superblock, so its starting
block effectively becomes the block number defined by
nilfs->ns_first_data_block.

While the segment range obtained by nilfs_get_segment_range() reflects
this adjustment, nilfs_get_segnum_of_block() does not (it returns 0
even for blocks preceding the first block in segment 0).

So, if we want to add a check beforehand, I think it would be better
to skip the operation if end_block is less than
nilfs->ns_first_data_block.

Thanks,
Ryusuke Konishi