linux-kernel - Re: [PATCH v2] pwm: Create device class for pwm channels

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, 5 Sep 2016 17:20:25 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     David Hsu <davidhsu@...gle.com>
Cc:     gregkh@...uxfoundation.org, sspatil@...gle.com,
        linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] pwm: Create device class for pwm channels

On Mon, Jul 18, 2016 at 03:14:50PM -0700, David Hsu wrote:
> Pwm channels don't send uevents when exported, this change adds the
> channels to a pwm class and set their device type to pwm_channel so
> uevents are sent.
> 
> To do this properly, the device names need to change to uniquely
> identify a channel.  This change is from pwmN to pwmchipM:N
> 
> Signed-off-by: David Hsu <davidhsu@...gle.com>
> ---
> v2: Use parent name instead of chip->base for channel naming.
> 
>  Documentation/pwm.txt |  6 ++++--
>  drivers/pwm/sysfs.c   | 18 +++++++++++++-----
>  2 files changed, 17 insertions(+), 7 deletions(-)

Forgot to mention one other thing that had come to my mind...

> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
[...]
> @@ -248,9 +254,11 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
>  
>  	export->child.release = pwm_export_release;
>  	export->child.parent = parent;
> +	export->child.type = &pwm_channel_type;
>  	export->child.devt = MKDEV(0, 0);
> +	export->child.class = &pwm_class;
>  	export->child.groups = pwm_groups;
> -	dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
> +	dev_set_name(&export->child, "%s:%u", dev_name(parent), pwm->hwpwm);

Using the colon as separator could possibly prevent the use of these
files in paths lists. Consider the case where somebody wanted to give a
list of PWM sysfs references:

	PWM_DEVICES=/sys/class/pwm/pwmchip0:0:/sys/class/pwm/pwmchip0:1

which might get parsed as four entries:

	- /sys/class/pwm/pwmchip0
	- 0
	- /sys/class/pwm/pwmchip0
	- 1

none of which point to a valid PWM. There are a couple of other
separators that might be possible, such as '-' or '.'.

Then again, looking at existing usage in sysfs it seems like the colon
is a fairly popular separator, so maybe I'm just being too paranoid.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)