linux-kernel - Re: [PATCH] gpiolib: of: fix bounds check for valid mask

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: <20220412070355.139507-1-andrei.lalaev@emlid.com>
Date:   Tue, 12 Apr 2022 10:03:58 +0300
From:   Andrei Lalaev <andrei.lalaev@...id.com>
To:     andy.shevchenko@...il.com
Cc:     linus.walleij@...aro.org, brgl@...ev.pl,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andrei Lalaev <andrei.lalaev@...id.com>
Subject: Re: [PATCH] gpiolib: of: fix bounds check for valid mask

On Mon, Apr 11, 2022 at 7:55 PM Andy Shevchenko <andy.shevchenko@...il.com> wrote:

...

> On top of that, it would be nice to be sure that at least all current
> in-kernel users (meaning all DTS provided so far by the kernel) do
> interpret it as start,size.

I checked the DTS and only 36 of them use "gpio-reserved-ranges".
The 33 of them use tuple with the second element that is less than the first.
So it means that the maintainers interpreted it as "start,size".

And only 3 of them use one tuple with the second element is greater than the first.
The list of this DTS:
    1. arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone.dtsi
        AngeloGioacchino Del Regno added it in the commit 9da65e441d4d
        ("arm64: dts: qcom: Add support for SONY Xperia X Performance / XZ / XZs (msm8996, Tone platform)")
        But in another commit 8b36c824b9a77 ("arm64: dts: qcom: sdm630-xperia-nile: Add all RPM and fixed regulators")
        he clearly interpreted it as "start,size" because the second element
        is less than the first.

    2. arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
        The same maintainer as for the previous DTS.
        He added "reserved-range" in the commit 122d2c5f31b6e
        ("arm64: dts: qcom: Add support for MSM8998 F(x)tec Pro1 QX1000")

    3. arch/arm64/boot/dts/qcom/sa8155p-adp.dts
        Bhupesh Sharma added it in the commit 12dd4ebda47ab
        ("arm64: dts: qcom: Fix usb entries for SA8155p adp board")

Should I ask these maintainers how they interpreted this property?

> Otherwise this will be an (unacceptable) ABI change and hence documentation
> would need to be fixed with variable names in the code

I want you to notice that "of_gpiochip_init_valid_mask" uses "bitmap_clear"
that clears "nbits" bits starting from the "start". So it can't be interpreted
as "start,end". That's why I think everyone interprets it as "start,size" because
it works like "start,size" and the documentation tells it is "start,size".