On Wed, May 08, 2024 at 08:49:39AM +0800, Edward Adam Davis wrote:
> On Tue, 7 May 2024 10:14:22 -0400, Kent Overstreet wrote:
> > > When got too small clean field, entry will never equal
> > > vstruct_end(&clean->field),
> > > the dead loop resulted in out of bounds access.
> > >
> > > Fixes: 12bf93a429c9 ("bcachefs: Add .to_text() methods for all superblock
> > > sections")
> > > Fixes: a37ad1a3aba9 ("bcachefs: sb-clean.c")
> > > Reported-and-tested-by:
> > > [email protected]
> > > Signed-off-by: Edward Adam Davis <[email protected]>
> >
> > I've already got a patch up for this - the validation was missing as
> > well.
> >
> > commit f39055220f6f98a180e3503fe05bbf9921c425c8
> > Author: Kent Overstreet <[email protected]>
> > Date: Sun May 5 22:28:00 2024 -0400
> >
> > bcachefs: Add missing validation for superblock section clean
> >
> > We were forgetting to check for jset entries that overrun the end of the
> > section - both in validate and to_text(); to_text() needs to be safe for
> > types that fail to validate.
> >
> > Reported-by: [email protected]
> > Signed-off-by: Kent Overstreet <[email protected]>
> >
> > diff --git a/fs/bcachefs/sb-clean.c b/fs/bcachefs/sb-clean.c
> > index 35ca3f138de6..194e55b11137 100644
> > --- a/fs/bcachefs/sb-clean.c
> > +++ b/fs/bcachefs/sb-clean.c
> > @@ -278,6 +278,17 @@ static int bch2_sb_clean_validate(struct bch_sb *sb,
> > return -BCH_ERR_invalid_sb_clean;
> > }
> >
> > + for (struct jset_entry *entry = clean->start;
> > + entry != vstruct_end(&clean->field);
> > + entry = vstruct_next(entry)) {
> > + if ((void *) vstruct_next(entry) > vstruct_end(&clean->field)) {
> > + prt_str(err, "entry type ");
> > + bch2_prt_jset_entry_type(err, le16_to_cpu(entry->type));
> > + prt_str(err, " overruns end of section");
> > + return -BCH_ERR_invalid_sb_clean;
> > + }
> > + }
> > +
> The original judgment here is sufficient, there is no need to add this
> section of inspection.
No, we need to be able to print things that failed to validate so that
we see what went wrong.
