Re: [External] Re: [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus

[Muchun Song]

On Sat, Jul 18, 2020 at 4:15 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> Muchun,
>
> Muchun Song <songmuchun@xxxxxxxxxxxxx> writes:
>
> > The get_option() maybe return 0, it means that the nr_cpus is
> > not initialized.
>
> Good catch, but see below.
>
> > Then we will use the stale nr_cpus to initialize
>
> We use nothing. Please describe your changes in technical neutral
> language.
>
> > the nr_cpu_ids. So fix it.
>
> 'So fix it.' is not much valuable information. What about:
>
>     Check the return value to prevent this.
>
> Hmm?

Looks fine to me. Thanks.

>
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > ---
> >  kernel/smp.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/kernel/smp.c b/kernel/smp.c
> > index 472c2b274c65..2a9a04acf123 100644
> > --- a/kernel/smp.c
> > +++ b/kernel/smp.c
> > @@ -634,8 +634,7 @@ static int __init nrcpus(char *str)
> >  {
> >       int nr_cpus;
> >
> > -     get_option(&str, &nr_cpus);
> > -     if (nr_cpus > 0 && nr_cpus < nr_cpu_ids)
> > +     if (get_option(&str, &nr_cpus) && nr_cpus > 0 && nr_cpus < nr_cpu_ids)
> >               nr_cpu_ids = nr_cpus;
> >
> >       return 0;
>
> get_option() can return 0 - 3:
>
>  *      0 - no int in string
>  *      1 - int found, no subsequent comma
>  *      2 - int found including a subsequent comma
>  *      3 - hyphen found to denote a range
>
> For this parameter exists only one valid format: '1 - int found, no comma',
> right?

Yeah.

>
> So why fixing it just half and why returning '0' aka success if the
> parameter is bogus?

Thanks. Will fix it.

>
> Thanks,
>
>         tglx



-- 
Yours,
Muchun