linux-kernel - Re: [PATCH 1/2] of/fdt: Allow architectures to override CONFIG

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] [day] [month] [year] [list]

Message-ID: <mhng-01382a17-8203-4155-9caf-20600f88bb37@palmer-si-x1c4>
Date:   Fri, 28 Sep 2018 18:44:59 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...ive.com>
To:     robh+dt@...nel.org
CC:     paul.burton@...s.com, linux-mips@...ux-mips.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        frowand.list@...il.com, jaedon.shin@...il.com, malat@...ian.org,
        stable@...r.kernel.org
Subject:     Re: [PATCH 1/2] of/fdt: Allow architectures to override CONFIG_CMDLINE logic

On Fri, 07 Sep 2018 13:29:03 PDT (-0700), robh+dt@...nel.org wrote:
> On Fri, Sep 7, 2018 at 1:55 PM Paul Burton <paul.burton@...s.com> wrote:
>>
>> The CONFIG_CMDLINE-related logic in early_init_dt_scan_chosen() falls
>> back to copying CONFIG_CMDLINE into boot_command_line/data if the DT has
>> a /chosen node but that node has no bootargs property or a bootargs
>> property of length zero.
>
> The Risc-V guys found a similar issue if chosen is missing[1]. I
> started a patch[2] to address that, but then looking at the different
> arches wasn't sure if I'd break something. I don't recall for sure,
> but it may have been MIPS that worried me.

IIRC we actually determined it didn't even work correctly on RISC-V, but I 
never actually got the time to figure out why and then forgot about it.  Sorry!

>
>> This is problematic for the MIPS architecture because we support
>> concatenating arguments from either the DT or the bootloader with those
>> from CONFIG_CMDLINE, but the behaviour of early_init_dt_scan_chosen()
>> gives us no way of knowing whether boot_command_line contains arguments
>> from DT or already contains CONFIG_CMDLINE. This can lead to us
>> concatenating CONFIG_CMDLINE with itself, duplicating command line
>> arguments which can be problematic (eg. for earlycon which will attempt
>> to register the same console twice & warn about it).
>
> If CONFIG_CMDLINE_EXTEND is set, you know it contains CONFIG_CMDLINE.
> But I guess part of the problem is MIPS using its own kconfig options.
>
>> Move the CONFIG_CMDLINE-related logic to a weak function that
>> architectures can provide their own version of, such that we continue to
>> use the existing logic for architectures where it's suitable but also
>> allow MIPS to override this behaviour such that the architecture code
>> knows when CONFIG_CMDLINE is used.
>
> More arch specific functions is not what I want. Really, all the
> cmdline manipulating logic doesn't belong in DT code, but it shouldn't
> be in the arch specific code either IMO. Really it should be some
> common kernel function which calls into the DT code to retrieve the DT
> bootargs and that's it. Then you can skip calling that kernel function
> if you really need non-standard handling.
>
> Perhaps you should consider filling DT bootargs with the cmdline from
> bootloader. IOW, make the legacy case look like the non-legacy case
> early, and then the kernel doesn't have to deal with both cases later
> on.
>
> Rob
>
> [1] https://lkml.org/lkml/2018/3/14/701
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/cmdline