Re: [PATCH] f2fs: bound the SIT/NAT bitmaps by the checkpoint pack

[Kaixuan Li]

On 10/14/26 14:14, Chao Yu wrote:
> It looks the overflow issue exists since the beginning, w/o c77ec61ca0a4,
> {sit,nat}_ver_bitmap_bytesize was not checked at all, c77ec61ca0a4 just
> added partial check, so how about:
>
> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure")
> Cc: stable@xxxxxxxxxx

Agreed, nothing bounded them against the allocation before c77ec61ca0a4
either. I'll send v2 with the Fixes tag changed, Cc stable, and your
Reviewed-by. Thanks for the review.

Kaixuan


On Wed, Oct 14, 2026 at 2:14 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 9/19/26 21:09, Kaixuan Li wrote:
> > sit_ver_bitmap_bytesize and nat_ver_bitmap_bytesize are used as memcpy
> > lengths into and out of sbi->ckpt, which is sized from cp_payload alone,
> > and nothing compares either length against that allocation.
> >
> > sanity_check_ckpt() has checked these two fields since c77ec61ca0a4, which
> > added them after a crafted image overran build_sit_info(). That check ties
> > each length to a segment count; it never compares either against the
> > allocation, which is the half this adds.
> >
> > Mounting over-reads in build_sit_info() and build_node_manager(), and the
> > first checkpoint writes the same lengths back through the same pointers,
> > so the over-read's contents reach the on-disk checkpoint. An unmount is
> > enough; the filesystem does not have to be used.
> >
> > Bound both ends the way __bitmap_ptr() lays them out. The three branches
> > differ, so a single sum of the two lengths would still admit a small
> > overflow when cp_payload is non-zero.
> >
> > Fixes: c77ec61ca0a4 ("f2fs: fix to do sanity check with {sit,nat}_ver_bitmap_bytesize")
>
> It looks the overflow issue exists since the beginning, w/o c77ec61ca0a4,
> {sit,nat}_ver_bitmap_bytesize was not checked at all, c77ec61ca0a4 just
> added partial check, so how about:
>
> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure")
> Cc: stable@xxxxxxxxxx
>
> > Signed-off-by: Kaixuan Li <kaixuanli0131@xxxxxxxxx>
> Otherwise, it looks good to me.
>
> Reviewed-by: Chao Yu <chao@xxxxxxxxxx>
>
>