| 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
| ||
|
Message-ID: <CAMRc=MdTuL9K4etfqM=BEkHy+KKWpT+JKHCo4iRdCX48gs8M8Q@mail.gmail.com>
Date: Wed, 18 Jun 2025 03:08:46 -0700
From: brgl@...ev.pl
To: Bjorn Andersson <andersson@...nel.org>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, Konrad Dybcio <konradybcio@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH] arm64: dts: qcom: add debug UART pins to reserved GPIO
ranges on RB2
On Wed, 18 Jun 2025 04:33:31 +0200, Bjorn Andersson <andersson@...nel.org> said:
> On Tue, Jun 17, 2025 at 01:28:41PM +0200, Bartosz Golaszewski wrote:
>> On Tue, Jun 17, 2025 at 5:18 AM Bjorn Andersson <andersson@...nel.org> wrote:
>> >
>> > On Mon, Jun 16, 2025 at 06:43:16PM +0200, Bartosz Golaszewski wrote:
>> > > On Mon, Jun 16, 2025 at 6:20 PM Konrad Dybcio
>> > > <konrad.dybcio@....qualcomm.com> wrote:
>> > > >
>> > > > On 6/16/25 4:33 PM, Bartosz Golaszewski wrote:
>> > > > > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>> > > > >
>> > > > > GPIO12 and GPIO13 are used for the debug UART and must not be available
>> > > > > to drivers or user-space. Add them to the gpio-reserved-ranges.
>> > > > >
>> > > > > Fixes: 8d58a8c0d930c ("arm64: dts: qcom: Add base qrb4210-rb2 board dts")
>> > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>> > > > > ---
>> > > >
>> > > > That also makes them unavailable to the kernel though, no?
>> > > >
>> > >
>> > > Yes. They could only be used by QUP - I2C or SPI #4 - on sm6115 but
>> > > none of these are used on RB2. I just noticed that my console froze
>> > > when I accidentally requested GPIO12 and figured that it makes sense
>> > > to make them unavailable. Let me know if this should be dropped.
>> > >
>> >
>> > I'm guessing that this would be a problem for any pin that is used for
>> > some other function. Should we instead prevent userspace from being able
>> > to request pins that are not in "gpio" pinmux state?
>> >
>>
>> That's supported by the "strict" flag in struct pinmux_ops. However
>> the two pins in question are muxed to GPIOs as far as the msm pinctrl
>> driver is concerned so it wouldn't help.
>
> This doesn't sound correct, the pins needs to be muxed to the qup in
> order for UART to work, so how can they show as "gpio" function?
>
There's no pinctrl-0 property in the uart4 node. But if we do the following:
diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index c8865779173ec..8c29440e9f43c 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -672,6 +672,18 @@ qup_i2c4_default: qup-i2c4-default-state {
bias-pull-up;
};
+ qup_uart4_default: qup-uart4-default-state {
+ qup_uart4_tx: tx-pins {
+ pins = "gpio12";
+ function = "qup4";
+ };
+
+ qup_uart4_rx: rx-pins {
+ pins = "gpio13";
+ function = "qup4";
+ };
+ };
+
qup_i2c5_default: qup-i2c5-default-state {
pins = "gpio14", "gpio15";
function = "qup5";
@@ -1565,6 +1577,8 @@ uart4: serial@...0000 {
reg = <0x0 0x04a90000 0x0 0x4000>;
clock-names = "se";
clocks = <&gcc GCC_QUPV3_WRAP0_S4_CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&qup_uart4_default>;
interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
interconnects = <&clk_virt
MASTER_QUP_CORE_0 RPM_ALWAYS_TAG
&clk_virt
SLAVE_QUP_CORE_0 RPM_ALWAYS_TAG>,
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c
b/drivers/pinctrl/qcom/pinctrl-msm.c
index 5c4687de1464a..e5c85d21e13c7 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -293,6 +293,7 @@ static const struct pinmux_ops msm_pinmux_ops = {
.get_function_groups = msm_get_function_groups,
.gpio_request_enable = msm_pinmux_request_gpio,
.set_mux = msm_pinmux_set_mux,
+ .strict = true,
};
static int msm_config_reg(struct msm_pinctrl *pctrl,
Then the problem on RB2 goes away.
>> Turning on the strict flag at
>> the global level of the pinctrl-msm driver would be risky though as it
>> would affect so many platforms, I'm sure it would break things. So IMO
>> it's either this change or let's drop it and leave it as is.
>>
>
> I share your concern, but the benefit sounds desirable. I think
> protecting the UART pins would set a precedence for filling that list
> with all kinds of pins, for all platforms, so lets give this some more
> thought,
>
I can only test this change on so many boards. We could give it a try, it's
early into the cycle so if we get this change into next now, we'd have some
time to see if anything breaks. I can send patches with the above changes
if you're alright with it.
Bart
Powered by blists - more mailing lists