linux-kernel - Re: [PATCH] pwm: sysfs: Set class on pwm devices

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, 1 Oct 2020 15:50:09 +0200
From:   Lars Poeschel <poeschel@...onage.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Thierry Reding <thierry.reding@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        "open list:PWM SUBSYSTEM" <linux-pwm@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pwm: sysfs: Set class on pwm devices

On Thu, Oct 01, 2020 at 01:24:49PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Oct 01, 2020 at 11:05:31AM +0200, Lars Poeschel wrote:
> > On Wed, Sep 30, 2020 at 11:41:46AM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > I added Greg Kroah-Hartman who I discussed this with via irc a bit to
> > > Cc:.
> > > 
> > > On Wed, Sep 30, 2020 at 11:20:56AM +0200, Lars Poeschel wrote:
> > > > thank you for your review!
> > > > 
> > > > On Wed, Sep 30, 2020 at 08:57:26AM +0200, Uwe Kleine-König wrote:
> > > > > On Tue, Sep 29, 2020 at 02:19:53PM +0200, poeschel@...onage.de wrote:
> > > > > > From: Lars Poeschel <poeschel@...onage.de>
> > > > > > 
> > > > > > This adds a class to exported pwm devices.
> > > > > > Exporting a pwm through sysfs did not yield udev events. The
> > > > > 
> > > > > I wonder what is your use-case here. This for sure also has a place to
> > > > > be mentioned in the commit log. I suspect there is a better way to
> > > > > accomplish you way.
> > > > 
> > > > Use-case is to be able to use a pwm from a non-root userspace process.
> > > > I use udev rules to adjust permissions.
> > > 
> > > Hmm, how do you trigger the export? Without being aware of all the
> > > details in the sysfs code I would expect that the exported stuff is
> > > available instantly once the write used to export the PWM is completed.
> > > So changing the permissions can be done directly after triggering the
> > > export in the same process.
> > 
> > The export is triggered through the userspace process itself. Why can it
> > do this ? Because there is another udev rule, that changes permissions
> > when a pwmchip appears.
> > Then I'd like to have the second udev rule, that changes permissions on
> > the freshly exported pwm. The userspace process can't do this.
> > You are right I could propably do everything from within udev: If a
> > pwmchip appears, export certain pwms and right away change their
> > permissions. It does not also not feel right. It'd require knowledge
> > from the userspace application to be mapped to udev.
> 
> The way the kernel code is now, yes, you will not have any way to
> trigger it by userspace as the kernel is creating a "raw" struct device
> that isn't assigned to anything.  That is what needs to be fixed here.

I did a first try with our approach.
I set the class of the child to its parent class. This does work and
create the directories right under /sys/pwm but because the child now
also inherits the dev_groups from the parent its directory also contain
export, unexport and npwm files, that don't apply for pwm's as soon a I
register the device to driver core.

Did we miss something or is there a way to avoid that ? I had a look at
device_add and saw that as soon as a class it set it's dev_groups get
exported through device_add_attrs.

> > > Out of interest: What do you use the pwm for? Isn't there a suitable
> > > kernel driver that can do the required stuff? Compared to the kernel-API
> > > the sysfs interface isn't atomic. Is this an annoyance?
> > 
> > Use-case is generating a voltage from the pwm. This voltage is used to
> > signal different states and does not change very often. This is
> > absolutely not annoying that this is not atomic. We just change the duty
> > cycle on the fly. Everything else is configured one time at startup.
> > I'd call what I need pwm-dac. I could not find a ready to use driver.
> > Maybe I could misuse some kernel driver for this. Maybe I could use
> > pwm-led or pwm-brightness or pwm-fan. Propably pwm-regulator could work,
> > there is even a userspace facing part for this, but this is not
> > devicetree ready.
> > ...and the worst, please don't blame me: The application is java, so
> > ioctl is a problem.
> 
> I thought java could do ioctls, otherwise how would it ever be able to
> talk to serial ports?

It is not impossible but it is horrible. java itself can access the
ports through normal file I/O, but it can not set control lines or
baudrate or anything. You need some C-code for this, that is not part of
the java vm itself and has to be called through something called JNI -
java native interface.

Regards,
Lars