On Mon, Aug 14, 2023 at 10:07 PM Paul Gortmaker
<paul.gortma...@windriver.com> 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 <paul.gortma...@windriver.com>
> ------------------
>
> 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 <adriancin...@gmail.com>
> > > ---
> > > 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(&non_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(&non_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
> > >
> >
> > >
> > >
> > >
> >
--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#12991):
https://lists.yoctoproject.org/g/linux-yocto/message/12991
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/leave/6687884/21656/624485779/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
