linux-kernel - Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working

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]

Date:   Thu, 28 Jul 2022 10:23:03 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     John Garry <john.garry@...wei.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Olof Johansson <olof@...om.net>, SoC Team <soc@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Vinod Koul <vkoul@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Jonathan Marek <jonathan@...ek.ca>
Subject: Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working

On Thu, Jul 28, 2022 at 10:06 AM John Garry <john.garry@...wei.com> wrote:
>
> trim list

Adding a few other people for slimbus.

> On 25/07/2022 13:51, Arnd Bergmann wrote:
> >>> CONFIG_SLIMBUS=m
> >> Config 5bd773242f75 added a kconfig "imply" on config SLIMBUS from
> >> config SOUNDWIRE_QCOM, and this happens to mean that we don't explicitly
> >> require config SLIMBUS enabled in the defconfig.
> > That 'imply' looks like it was added to solve a problem using the old 'imply'
> > semantics that are not what this keyword does today. I suspect it's still
> > broken when CONFIG=SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m,
> > and the correct fix is to use a dependency like
> >
> >        depends on SLIMBUS || !SLIMBUS
> >
> > so the defconfig symbol should stay.
> >
>
> JFYI, building for CONFIG_SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m is ok.
> The driver has a runtime check for CONFIG_SLIMBUS in qcom_swrm_probe().
>
> That implementation seems consistent with guidance in
> kconfig-language.rst - so do you think that there is still a problem?

I see Vinod added the IS_REACHABLE() check, which makes it build, but
I think both the 'imply' and the IS_REACHABLE() are mistakes here, as they
are almost everywhere else.

>From what I understand, the driver can actually use two different
back-ends, with slimbus being one of them. In the configuration
with CONFIG_SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m,
I think you can end up with a slimbus device being detected at
runtime, which gets passed to the built-in qcom driver, but that driver
then misdetects the bus because it is not linked against the
slimbus module. It then uses the incorrect code path, causing
unintended behavior.

The 'depends on SLIMBUS || !SLIMBUS' dependency would completely
avoid this issue and make the driver work correctly in each configuration.

        Arnd