* [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus
@ 2020-07-16 7:04 Muchun Song
2020-07-17 20:15 ` Thomas Gleixner
2020-07-22 9:12 ` [tip: sched/core] " tip-bot2 for Muchun Song
0 siblings, 2 replies; 4+ messages in thread
From: Muchun Song @ 2020-07-16 7:04 UTC (permalink / raw)
To: peterz, tglx, mingo, bigeasy, namit; +Cc: linux-kernel, Muchun Song
The get_option() maybe return 0, it means that the nr_cpus is
not initialized. Then we will use the stale nr_cpus to initialize
the nr_cpu_ids. So fix it.
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
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;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus
2020-07-16 7:04 [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus Muchun Song
@ 2020-07-17 20:15 ` Thomas Gleixner
2020-07-18 2:29 ` [External] " Muchun Song
2020-07-22 9:12 ` [tip: sched/core] " tip-bot2 for Muchun Song
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2020-07-17 20:15 UTC (permalink / raw)
To: Muchun Song, peterz, mingo, bigeasy, namit; +Cc: linux-kernel, Muchun Song
Muchun,
Muchun Song <songmuchun@bytedance.com> 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?
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
> 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?
So why fixing it just half and why returning '0' aka success if the
parameter is bogus?
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus
2020-07-17 20:15 ` Thomas Gleixner
@ 2020-07-18 2:29 ` Muchun Song
0 siblings, 0 replies; 4+ messages in thread
From: Muchun Song @ 2020-07-18 2:29 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Peter Zijlstra, mingo, bigeasy, namit, LKML
On Sat, Jul 18, 2020 at 4:15 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Muchun,
>
> Muchun Song <songmuchun@bytedance.com> 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@bytedance.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip: sched/core] smp: Fix a potential usage of stale nr_cpus
2020-07-16 7:04 [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus Muchun Song
2020-07-17 20:15 ` Thomas Gleixner
@ 2020-07-22 9:12 ` tip-bot2 for Muchun Song
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot2 for Muchun Song @ 2020-07-22 9:12 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Muchun Song, Peter Zijlstra (Intel), x86, LKML
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 589343569d7b58a64ec2446e6686c8e79aea7fcf
Gitweb: https://git.kernel.org/tip/589343569d7b58a64ec2446e6686c8e79aea7fcf
Author: Muchun Song <songmuchun@bytedance.com>
AuthorDate: Thu, 16 Jul 2020 15:04:57 +08:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 22 Jul 2020 10:22:04 +02:00
smp: Fix a potential usage of stale nr_cpus
The get_option() maybe return 0, it means that the nr_cpus is
not initialized. Then we will use the stale nr_cpus to initialize
the nr_cpu_ids. So fix it.
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200716070457.53255-1-songmuchun@bytedance.com
---
kernel/smp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/smp.c b/kernel/smp.c
index aa17eed..d0ae8eb 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;
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-22 9:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 7:04 [PATCH RESEND] smp: Fix a potential usage of stale nr_cpus Muchun Song
2020-07-17 20:15 ` Thomas Gleixner
2020-07-18 2:29 ` [External] " Muchun Song
2020-07-22 9:12 ` [tip: sched/core] " tip-bot2 for Muchun Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).