linux-kernel - Re: [PATCH v3 2/3] pinctrl: core: configure pinmux from pins debug file

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:   Mon, 24 May 2021 19:28:24 +0200 (CEST)
From:   Dario Binacchi <dariobin@...ero.it>
To:     Vladimir Zapolskiy <vz@...ia.com>, linux-kernel@...r.kernel.org
Cc:     Tony Lindgren <tony@...mide.com>,
        Drew Fustini <drew@...gleboard.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-gpio@...r.kernel.org
Subject: Re: [PATCH v3 2/3] pinctrl: core: configure pinmux from pins debug
 file

Hi Vladimir,

> Il 21/05/2021 08:44 Vladimir Zapolskiy <vz@...ia.com> ha scritto:
> 
>  
> Hello Dario,
> 
> On 5/20/21 11:27 PM, Dario Binacchi wrote:
> > The MPUs of some architectures (e.g AM335x) must be in privileged
> > operating mode to write on the pinmux registers. In such cases, where
> > writes will not work from user space, now it can be done from the pins
> 
> user space has no connection to the problem you're trying to solve.
> 
> Please provide a reasonable rationale for adding a new interface, thank
> you in advance.
> 
> > debug file if the platform driver exports the pin_dbg_set() helper among
> > the registered operations.
> > 
> > Signed-off-by: Dario Binacchi <dariobin@...ero.it>
> 
> I strongly object against this new interface.
> 
> As Andy've already mentioned you have to operate with defined pin groups
> and functions, and so far you create an interface with an option to
> disasterous misusage, it shall be avoided, because there are better
> options.
> 
> What's the issue with a regular declaration of pin groups and functions
> on your SoC? When it's done, you can operate on this level of abstraction,
> there is absolutely no need to add the proposed low-level debug interface.
> 

I quote Drew's words: 

"I think it could be helpful to be able to set the conf_<module>_<pin>  
registers directly through debugfs as I can imagine situations where it would 
be useful for testing. It is a bit dangerous as the person using it has to be 
careful not to change the wrong bits, but they would need to have debugfs mounted 
and permissions to write to it."

"Bits 6:3 are related to what this subsystem would refer to as pin conf
such as slew, input enable and bias. Thus it might make sense to expose
something like a select-pinconf file to activate pin conf settings from
userspace."
 
>From the emails exchanged I seem to have understood that there is no way to 
reconfigure slew rate, pull up / down and other properties on the fly. 
In the kernel version 4.1.6 that I am using on my custom board, I have fixed
the commit f07512e615dd ("pinctrl/pinconfig: add debug interface"). However,
this feature was later removed (https://lore.kernel.org/patchwork/patch/1033755/).
The patches I've submitted implement some sort of devmem for pinmux. It too can 
be used in a dangerous way, but it exists and it is used.

Anyway, the implementation may be wrong but it does highlight a feature that 
can be useful in testing or prototyping boards but is not present in the kernel.
Can we then find a solution that is right for everyone?

Thanks and regards, 
Dario

> --
> Best wishes,
> Vladimir