Re: [f2fs-dev] [PATCH] fsck.f2fs: fix sanity check logic for cp_payload

[Jaegeuk Kim <jaegeuk@kernel.org>]

On 04/10, Juhyung Park wrote:
> Hi Jaegeuk,
> 
> Thanks for the timely merge.
> 
> Can we have a new f2fs-tools tag with this commit in place?
> Alex (the original bug reporter) would like to see this incorporated
> in the next version of Ubuntu (and Debian), and having a new tag is
> much easier to convince them to pack a new tag than having one patch
> backported.
> 
> In a practical scenario, a 2TB partition formatted with mkfs.f2fs -i
> simply doesn't boot under Ubuntu as fsck.f2fs inside the initramfs
> returns an error.
> 
> This patch fixes that and therefore allows the users to boot normally
> without manually excluding the f2fs partition to be checked during
> boot.

Thank you for heads up. I released v1.16.0. Please check. :)

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/tag/?h=v1.16.0

> 
> Thanks. Regards
> 
> On Tue, Apr 4, 2023 at 2:54 PM Juhyung Park <qkrwngud825@gmail.com> wrote:
> >
> > cp_payload is set differently [1] when extended node bitmap feature is
> > enabled. Commit b79c3ba4ea9d broke fsck on f2fs file systems created on
> > 2+ TB device with extended node bitmap feature enabled.
> >
> > As the sanity check is for checking overflows, fix this to assume the max
> > possible cp_payload size under the extended node bitmap.
> >
> > Link: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/tree/mkfs/f2fs_format.c?h=v1.15.0#n372 [1]
> > Fixes: b79c3ba4ea9d ("fsck.f2fs: sanity check cp_payload before reading checkpoint")
> > Reported-by: Alexander Koskovich <akoskovich@pm.me>
> > Signed-off-by: Juhyung Park <qkrwngud825@gmail.com>
> > ---
> >  fsck/mount.c      | 2 +-
> >  include/f2fs_fs.h | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fsck/mount.c b/fsck/mount.c
> > index 2b26701..df0314d 100644
> > --- a/fsck/mount.c
> > +++ b/fsck/mount.c
> > @@ -1208,7 +1208,7 @@ int get_valid_checkpoint(struct f2fs_sb_info *sbi)
> >         int ret;
> >
> >         cp_payload = get_sb(cp_payload);
> > -       if (cp_payload > F2FS_BLK_ALIGN(MAX_SIT_BITMAP_SIZE))
> > +       if (cp_payload > F2FS_BLK_ALIGN(MAX_CP_PAYLOAD))
> >                 return -EINVAL;
> >
> >         cp_blks = 1 + cp_payload;
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index 333ae07..f890634 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -1168,6 +1168,10 @@ static_assert(sizeof(struct f2fs_nat_block) == 4095, "");
> >  #define MAX_SIT_BITMAP_SIZE    (SEG_ALIGN(SIZE_ALIGN(F2FS_MAX_SEGMENT, \
> >                                                 SIT_ENTRY_PER_BLOCK)) * \
> >                                                 c.blks_per_seg / 8)
> > +#define MAX_CP_PAYLOAD         (SEG_ALIGN(SIZE_ALIGN(UINT32_MAX, NAT_ENTRY_PER_BLOCK)) * \
> > +                                               DEFAULT_NAT_ENTRY_RATIO / 100 * \
> > +                                               c.blks_per_seg / 8 + \
> > +                                               MAX_SIT_BITMAP_SIZE - MAX_BITMAP_SIZE_IN_CKPT)
> >
> >  /*
> >   * Note that f2fs_sit_entry->vblocks has the following bit-field information.
> > --
> > 2.40.0
> >


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel