linux-kernel - Re: gpiolib: add export/unexport by gpio name

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] [day] [month] [year] [list]

Message-Id: <200908041358.29400.david-b@pacbell.net>
Date:	Tue, 4 Aug 2009 13:58:29 -0700
From:	David Brownell <david-b@...bell.net>
To:	ben@...ff.org
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: gpiolib: add export/unexport by gpio name

On Tuesday 04 August 2009, ben@...ff.org wrote:
> Add the facility to export/unexport a gpio by the name assigned to it
> as well as the number.

I guess I'm still a bit puzzled by the notion of adding
second/parallel naming scheme for GPIOs.  As a rule, such
duplication is undesirable ...

But assuming such things are OK, this patch has a couple
technical problems.


> Signed-off-by: Ben Dooks <ben@...tec.co.uk>
> Cc: David Brownell <dbrownell@...rs.sourceforge.net>
> 
> ---
>  drivers/gpio/gpiolib.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 66 insertions(+), 4 deletions(-)
> 
> Index: b/drivers/gpio/gpiolib.c
> ===================================================================
> --- a/drivers/gpio/gpiolib.c	2009-08-04 11:53:53.000000000 +0100
> +++ b/drivers/gpio/gpiolib.c	2009-08-04 11:54:12.000000000 +0100
> @@ -364,6 +364,64 @@ static const struct attribute_group gpio
>  	.attrs = (struct attribute **) gpiochip_attrs,
>  };
>  
> +static int search_names(char **names, const char *match, int ngpio)
> +{
> +	int len = strlen(match);
> +	char *m;
> +	int ptr;
> +
> +	/* if cr terminated, remove */
> +	if (match[len-1] == '\n')
> +		len--;
> +
> +	if (names) {
> +		for (ptr = 0; ptr < ngpio; ptr++) {
> +			m = names[ptr];
> +			if (!m)
> +				continue;
> +
> +			if (strlen(m) != len)
> +				continue;
> +
> +			if (strncmp(m, match, len) == 0)
> +				return ptr;
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +static long gpio_from_string(const char *buf)
> +{
> +	struct gpio_chip *chip;
> +	struct gpio_desc *desc;
> +	long status;
> +	long gpio;
> +	int ptr, off;
> +
> +	status = strict_strtol(buf, 0, &gpio);
> +	if (status < 0) {
> +		for (ptr = 0; ptr < ARCH_NR_GPIOS && status < 0;) {

Nothing is guarding against
 (a) duplicate chip->names[] entries within or between chips,
 (b) concurrent addition/removal of gpio_chip instances

For (a) maybe the answer is to detect them at chip registration
time ... then either fail, or else warn then null out the dup.

For (b) it seems that gpio_lock is required.

> +			desc = gpio_desc + ptr;
> +			chip = desc->chip;
> +
> +			if (!chip) {
> +				ptr++;
> +				continue;
> +			}
> +
> +			off = search_names(chip->names, buf, chip->ngpio);
> +			if (off >= 0)
> +				status = ptr + off;
> +
> +			ptr += chip->ngpio;
> +		}
> +	} else
> +		status = gpio;
> +
> +	return status;
> +}
> +
>  /*
>   * /sys/class/gpio/export ... write-only
>   *	integer N ... number of GPIO to export (full access)
> @@ -375,9 +433,11 @@ static ssize_t export_store(struct class
>  	long	gpio;
>  	int	status;
>  
> -	status = strict_strtol(buf, 0, &gpio);
> -	if (status < 0)
> +	gpio = gpio_from_string(buf);
> +	if (gpio < 0) {
> +		status = gpio;
>  		goto done;
> +	}
>  
>  	/* No extra locking here; FLAG_SYSFS just signifies that the
>  	 * request and export were done by on behalf of userspace, so
> @@ -405,9 +465,11 @@ static ssize_t unexport_store(struct cla
>  	long	gpio;
>  	int	status;
>  
> -	status = strict_strtol(buf, 0, &gpio);
> -	if (status < 0)
> +	gpio = gpio_from_string(buf);
> +	if (gpio < 0) {
> +		status = gpio;
>  		goto done;
> +	}
>  
>  	status = -EINVAL;
>  
> 
> -- 
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/