On Sat, Sep 21, 2024 at 11:24:54PM GMT, Mohammed Anees wrote: > In this patch, I added a proper bounds check to avoid a potential NULL > pointer dereference when opt->choices[v] is accessed in the > bch2_opt_to_text function. This ensures that v is within the valid > range of choices. > > Reported-and-tested-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=37186860aa7812b331d5 > Signed-off-by: Mohammed Anees <[email protected]> > --- > > v2: > - Replaced pr_err with prt_printf as suggested. > - Added the bounds check on opt->choices before accessing it. > > fs/bcachefs/opts.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c > index e10fc1da7..9bb15694d 100644 > --- a/fs/bcachefs/opts.c > +++ b/fs/bcachefs/opts.c > @@ -420,8 +420,12 @@ void bch2_opt_to_text(struct printbuf *out, > case BCH_OPT_STR: > if (flags & OPT_SHOW_FULL_LIST) > prt_string_option(out, opt->choices, v); > - else > - prt_str(out, opt->choices[v]); > + else { > + if (v < opt->min || v >= opt->max - 1) > + prt_printf(out, "(invalid option %lli)", v); > + else > + prt_str(out, opt->choices[v]); > + }
For consistency, the bounds check should come first (applying in the OPT_SHOW_FULL_LIST case as well) - also, kill the nested if clauses, this all works with chained if else. Sorry for dribbling out the review feedback, that should be everything...
