Re: [linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in nohz_full/isolcpus setup
On Mon, Aug 14, 2023 at 10:07 PM Paul Gortmaker
wrote:
>
> [Re: [linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in
> nohz_full/isolcpus setup] On 26/06/2023 (Mon 11:05) Paul Gortmaker wrote:
>
> > [[linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in
> > nohz_full/isolcpus setup] On 25/06/2023 (Sun 18:50) Adrian Cinal via
> > lists.yoctoproject.org wrote:
> >
> > > Fix reading uninitialized cpumask and using it to validate the nohz_full=
> > > and isolcpus= kernel command line parameters.
> > >
> > > An older version of a patch from lkml was incorporated into linux-yocto,
> > > whereas a newer, rebased version was later published. See:
> > > https://lore.kernel.org/lkml/20220221182009.1283-1-paul.gortma...@windriver.com/
> >
> > Let me remind myself of what got merged upstream and what didn't and
> > why, and I'll follow up shortly with a Yocto specific update.
>
> Sorry for the delayed reply. The commit log kind of confused me for a
> while until I had a quiet moment to get the cobwebs out of my head and
> realize what happened.
>
> Your fix is correct. The v6.1 (and v6.4) kernels are performing the
> sanity tests on uninitialized memory and hence isolcpus= can randomly
> reject perfectly valid inputs. Same for nohz_full= it seems.
>
> I'd suggest we augment the commit log with this:
>
> --
> PG: To be more clear as to what happened here - it isn't a broken older
> patch from lkml integrated into linux-yocto. It is a carry forward of
> a correct commit from the v5.15 linux-yocto kernel:
>
> https://git.yoctoproject.org/linux-yocto/commit/?id=97c96388922
>
> ...in which case the sanity checks are properly *after* the allocation
> and processing of the bootargs into the cpumask.
>
> However, it seems patch (or wiggle?) apparently decided to put the
> sanity checks *before* the population of the cpumask during the
> carry-forward and generation of the new v6.1 kernel. Meaning they are
> validating uninitialized memory and hence nohz_full= and isolcpus= are
> subject to random failures even for valid input ranges.
>
> Acked-by: Paul Gortmaker
> --
>
> Bruce - both carry-forwards -- the v6.1 [d81fac6e842] and v6.4 kernels
> [23b162bc3058] have this issue. The commit IDs above are in their
> respective standard/base version and hence this fix will have to also
> land there and be merged out to -rt and and all BSPs etc etc.
>
> The copies in the yocto-kernel-cache also have the sanity checks above
> the actual cpulist_parse(str, non_housekeeping_mask) which populates the
> cpumask with the data to be validated and hence are also broken.
>
> https://git.yoctoproject.org/yocto-kernel-cache/tree/features/clear_warn_once/sched-isolation-really-align-nohz_full-with-rcu_nocb.patch?h=yocto-6.1
> https://git.yoctoproject.org/yocto-kernel-cache/tree/features/clear_warn_once/sched-isolation-really-align-nohz_full-with-rcu_nocb.patch?h=yocto-6.4
>
> Thanks to Adrian for tracking this down and sending the fix!
And thanks for the excellent detail Paul!
I've updated the kernel-cache commits, and have merged Adrian's
patches to the kernel branches.
My next series will include the SRCREV bumps for the change.
Bruce
>
> Paul.
> --
>
> >
> > Thanks,
> > Paul.
> > --
> >
> > >
> > > Signed-off-by: Adrian Cinal
> > > ---
> > > kernel/sched/isolation.c | 12 ++--
> > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> > > index 73386019efcb..b97d6e05013d 100644
> > > --- a/kernel/sched/isolation.c
> > > +++ b/kernel/sched/isolation.c
> > > @@ -119,6 +119,12 @@ static int __init housekeeping_setup(char *str,
> > > unsigned long flags)
> > > }
> > > }
> > >
> > > + alloc_bootmem_cpumask_var(_housekeeping_mask);
> > > + if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> > > + pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
> > > range\n");
> > > + goto free_non_housekeeping_mask;
> > > + }
> > > +
> > > if (!cpumask_subset(non_housekeeping_mask, cpu_possible_mask)) {
> > > pr_info("housekeeping: kernel parameter 'nohz_full=' or
> > > 'isolcpus=' contains nonexistent CPUs.\n");
> > > cpumask_and(non_housekeeping_mask, cpu_possible_mask,
> > > @@ -128,12 +134,6 @@ static int __init housekeeping_setup(char *str,
> > > unsigned long flags
Re: [linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in nohz_full/isolcpus setup
[Re: [linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in
nohz_full/isolcpus setup] On 26/06/2023 (Mon 11:05) Paul Gortmaker wrote:
> [[linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in
> nohz_full/isolcpus setup] On 25/06/2023 (Sun 18:50) Adrian Cinal via
> lists.yoctoproject.org wrote:
>
> > Fix reading uninitialized cpumask and using it to validate the nohz_full=
> > and isolcpus= kernel command line parameters.
> >
> > An older version of a patch from lkml was incorporated into linux-yocto,
> > whereas a newer, rebased version was later published. See:
> > https://lore.kernel.org/lkml/20220221182009.1283-1-paul.gortma...@windriver.com/
>
> Let me remind myself of what got merged upstream and what didn't and
> why, and I'll follow up shortly with a Yocto specific update.
Sorry for the delayed reply. The commit log kind of confused me for a
while until I had a quiet moment to get the cobwebs out of my head and
realize what happened.
Your fix is correct. The v6.1 (and v6.4) kernels are performing the
sanity tests on uninitialized memory and hence isolcpus= can randomly
reject perfectly valid inputs. Same for nohz_full= it seems.
I'd suggest we augment the commit log with this:
--
PG: To be more clear as to what happened here - it isn't a broken older
patch from lkml integrated into linux-yocto. It is a carry forward of
a correct commit from the v5.15 linux-yocto kernel:
https://git.yoctoproject.org/linux-yocto/commit/?id=97c96388922
...in which case the sanity checks are properly *after* the allocation
and processing of the bootargs into the cpumask.
However, it seems patch (or wiggle?) apparently decided to put the
sanity checks *before* the population of the cpumask during the
carry-forward and generation of the new v6.1 kernel. Meaning they are
validating uninitialized memory and hence nohz_full= and isolcpus= are
subject to random failures even for valid input ranges.
Acked-by: Paul Gortmaker
--
Bruce - both carry-forwards -- the v6.1 [d81fac6e842] and v6.4 kernels
[23b162bc3058] have this issue. The commit IDs above are in their
respective standard/base version and hence this fix will have to also
land there and be merged out to -rt and and all BSPs etc etc.
The copies in the yocto-kernel-cache also have the sanity checks above
the actual cpulist_parse(str, non_housekeeping_mask) which populates the
cpumask with the data to be validated and hence are also broken.
https://git.yoctoproject.org/yocto-kernel-cache/tree/features/clear_warn_once/sched-isolation-really-align-nohz_full-with-rcu_nocb.patch?h=yocto-6.1
https://git.yoctoproject.org/yocto-kernel-cache/tree/features/clear_warn_once/sched-isolation-really-align-nohz_full-with-rcu_nocb.patch?h=yocto-6.4
Thanks to Adrian for tracking this down and sending the fix!
Paul.
--
>
> Thanks,
> Paul.
> --
>
> >
> > Signed-off-by: Adrian Cinal
> > ---
> > kernel/sched/isolation.c | 12 ++--
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> > index 73386019efcb..b97d6e05013d 100644
> > --- a/kernel/sched/isolation.c
> > +++ b/kernel/sched/isolation.c
> > @@ -119,6 +119,12 @@ static int __init housekeeping_setup(char *str,
> > unsigned long flags)
> > }
> > }
> >
> > + alloc_bootmem_cpumask_var(_housekeeping_mask);
> > + if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> > + pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
> > range\n");
> > + goto free_non_housekeeping_mask;
> > + }
> > +
> > if (!cpumask_subset(non_housekeeping_mask, cpu_possible_mask)) {
> > pr_info("housekeeping: kernel parameter 'nohz_full=' or
> > 'isolcpus=' contains nonexistent CPUs.\n");
> > cpumask_and(non_housekeeping_mask, cpu_possible_mask,
> > @@ -128,12 +134,6 @@ static int __init housekeeping_setup(char *str,
> > unsigned long flags)
> > if (cpumask_empty(non_housekeeping_mask)) {
> > pr_info("housekeeping: kernel parameter 'nohz_full=' or
> > 'isolcpus=' has no valid CPUs.\n");
> > free_bootmem_cpumask_var(non_housekeeping_mask);
> > - return 0;
> > - }
> > -
> > - alloc_bootmem_cpumask_var(_housekeeping_mask);
> > - if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> > - pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
> > range\n");
> > goto free_non_housekeeping_mask;
> > }
> >
> > --
> > 2.41.0
> >
>
> >
> >
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#12975):
https://lists.yoctoproject.org/g/linux-yocto/message/12975
Mute This Topic: https://lists.yoctoproject.org/mt/99772129/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in nohz_full/isolcpus setup
[[linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in
nohz_full/isolcpus setup] On 25/06/2023 (Sun 18:50) Adrian Cinal via
lists.yoctoproject.org wrote:
> Fix reading uninitialized cpumask and using it to validate the nohz_full=
> and isolcpus= kernel command line parameters.
>
> An older version of a patch from lkml was incorporated into linux-yocto,
> whereas a newer, rebased version was later published. See:
> https://lore.kernel.org/lkml/20220221182009.1283-1-paul.gortma...@windriver.com/
Let me remind myself of what got merged upstream and what didn't and
why, and I'll follow up shortly with a Yocto specific update.
Thanks,
Paul.
--
>
> Signed-off-by: Adrian Cinal
> ---
> kernel/sched/isolation.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 73386019efcb..b97d6e05013d 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -119,6 +119,12 @@ static int __init housekeeping_setup(char *str, unsigned
> long flags)
> }
> }
>
> + alloc_bootmem_cpumask_var(_housekeeping_mask);
> + if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> + pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
> range\n");
> + goto free_non_housekeeping_mask;
> + }
> +
> if (!cpumask_subset(non_housekeeping_mask, cpu_possible_mask)) {
> pr_info("housekeeping: kernel parameter 'nohz_full=' or
> 'isolcpus=' contains nonexistent CPUs.\n");
> cpumask_and(non_housekeeping_mask, cpu_possible_mask,
> @@ -128,12 +134,6 @@ static int __init housekeeping_setup(char *str, unsigned
> long flags)
> if (cpumask_empty(non_housekeeping_mask)) {
> pr_info("housekeeping: kernel parameter 'nohz_full=' or
> 'isolcpus=' has no valid CPUs.\n");
> free_bootmem_cpumask_var(non_housekeeping_mask);
> - return 0;
> - }
> -
> - alloc_bootmem_cpumask_var(_housekeeping_mask);
> - if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> - pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
> range\n");
> goto free_non_housekeeping_mask;
> }
>
> --
> 2.41.0
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#12803):
https://lists.yoctoproject.org/g/linux-yocto/message/12803
Mute This Topic: https://lists.yoctoproject.org/mt/99772129/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[linux-yocto] [PATCH] kernel/sched: Fix uninitialized read in nohz_full/isolcpus setup
Fix reading uninitialized cpumask and using it to validate the nohz_full=
and isolcpus= kernel command line parameters.
An older version of a patch from lkml was incorporated into linux-yocto,
whereas a newer, rebased version was later published. See:
https://lore.kernel.org/lkml/20220221182009.1283-1-paul.gortma...@windriver.com/
Signed-off-by: Adrian Cinal
---
kernel/sched/isolation.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 73386019efcb..b97d6e05013d 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -119,6 +119,12 @@ static int __init housekeeping_setup(char *str, unsigned
long flags)
}
}
+ alloc_bootmem_cpumask_var(_housekeeping_mask);
+ if (cpulist_parse(str, non_housekeeping_mask) < 0) {
+ pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
range\n");
+ goto free_non_housekeeping_mask;
+ }
+
if (!cpumask_subset(non_housekeeping_mask, cpu_possible_mask)) {
pr_info("housekeeping: kernel parameter 'nohz_full=' or
'isolcpus=' contains nonexistent CPUs.\n");
cpumask_and(non_housekeeping_mask, cpu_possible_mask,
@@ -128,12 +134,6 @@ static int __init housekeeping_setup(char *str, unsigned
long flags)
if (cpumask_empty(non_housekeeping_mask)) {
pr_info("housekeeping: kernel parameter 'nohz_full=' or
'isolcpus=' has no valid CPUs.\n");
free_bootmem_cpumask_var(non_housekeeping_mask);
- return 0;
- }
-
- alloc_bootmem_cpumask_var(_housekeeping_mask);
- if (cpulist_parse(str, non_housekeeping_mask) < 0) {
- pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU
range\n");
goto free_non_housekeeping_mask;
}
--
2.41.0
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#12797):
https://lists.yoctoproject.org/g/linux-yocto/message/12797
Mute This Topic: https://lists.yoctoproject.org/mt/99772129/21656
Group Owner: linux-yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
