linux-kernel - Re: [PATCH] of: fdt: Honor CONFIG_CMDLINE* even without /chosen node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221213152920.GA1025576-robh@kernel.org>
Date:   Tue, 13 Dec 2022 09:29:20 -0600
From:   Rob Herring <robh@...nel.org>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>
Cc:     devicetree@...r.kernel.org, Frank Rowand <frowand.list@...il.com>,
        linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...nel.org>,
        Nikita Shubin <nikita.shubin@...uefel.me>,
        Hartley Sweeten <hsweeten@...ionengravers.com>,
        Lukasz Majewski <lukma@...x.de>,
        Linus Walleij <linusw@...nel.org>
Subject: Re: [PATCH] of: fdt: Honor CONFIG_CMDLINE* even without /chosen node

On Tue, Dec 13, 2022 at 09:51:33AM +0100, Linus Walleij wrote:
> On Mon, Dec 12, 2022 at 7:01 AM Alexander Sverdlin
> <alexander.sverdlin@...il.com> wrote:
> 
> > I do not read a strict requirement on /chosen node in either ePAPR or in
> > Documentation/devicetree. Help text for CONFIG_CMDLINE and
> > CONFIG_CMDLINE_EXTEND doesn't make their behavior explicitly dependent on
> > the presence of /chosen or the presense of /chosen/bootargs.
> >
> > However the early check for /chosen and bailing out in
> > early_init_dt_scan_chosen() skips CONFIG_CMDLINE handling which is not
> > really related to /chosen node or the particular method of passing cmdline
> > from bootloader.
> >
> > This leads to counterintuitive combinations (assuming
> > CONFIG_CMDLINE_EXTEND=y):
> >
> > a) bootargs="foo", CONFIG_CMDLINE="bar" => cmdline=="foo bar"
> > b) /chosen missing, CONFIG_CMDLINE="bar" => cmdline==""
> > c) bootargs="", CONFIG_CMDLINE="bar" => cmdline==" bar"
> >
> > Move CONFIG_CMDLINE handling outside of early_init_dt_scan_chosen() so that
> > cases b and c above result in the same cmdline.
> >
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> 
> Excellent debugging Alexander!
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> 
> I also think this should go to stable.

We have to be careful there. This could change behavior on a working 
system. A system taking the cmdline entirely from a built kernel and 
no initrd is going to be pretty customized already, I think they can 
carry a patch. What platform is this anyways?

This has actually been known for some time[1][2]. My concern in the past 
(besides wanting all the cmdline manipulation being common) was MIPS. 
MIPS in particular has lots of sources for cmdline and ways to combine 
it. However, MIPS has since stopped using this code and does their own 
parsing (not great either IMO).

Rob

[1] https://lore.kernel.org/all/CAL_Jsq+CgxWbCMz_qwLbMJS3fYwKyCMBGVS501-5ShXywyDAXA@mail.gmail.com/
[2] https://lore.kernel.org/all/CAL_Jsq+n4e5_ptuh89CJibViGS_bgHz0A+Ki-uwtcGU38+mHXQ@mail.gmail.com/