linux-kernel - Re: PIXIS gpio controller and gpio flags

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:	Sat, 19 Jul 2008 14:08:01 -0700 (PDT)
From:	Trent Piepho <tpiepho@...escale.com>
To:	Anton Vorontsov <avorontsov@...mvista.com>
cc:	Grant Likely <grant.likely@...retlab.ca>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org
Subject: Re: PIXIS gpio controller and gpio flags

On Fri, 18 Jul 2008, Anton Vorontsov wrote:
> +int px_gpio_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
> +               const void *gpio_spec)
> +{
> +     if (gpio[1] & PX_GPIO_FLAG_ACTIVE_LOW)
> +             px_gc->active_low |= pin2mask(*gpio);

You have a race here.  What if px_gpio_xlate() is called at the same time
for different gpios?  This is an easy one to fix, since you can use the
atomic bitops.

It doesn't look like you have any way to unset the active low flag.  What if
I unload the leds-gpio driver (or another gpio user) and then try to use the
gpio with something else?  The active low flag is stuck on!  It doesn't show
in sysfs or debugfs either.  That could be very confusing.

I also wonder if it's ok to have the xlate function do flag setting?

of_get_property() just gets the property, it doesn't allocate it.  Same with
of_get_address() and of_get_pci_address(), they don't actually allocate or
map an address, they just get the value.  of_get_gpio() doesn't allocate the
gpio, that gets done later with gpio_request().  It seems like what it's
supposed to do is just get the translated value of the gpio property.

Except, your pixis gpio xlate function sets the gpio's flags.  What if one
wants to just look up a gpio number, but not allocate it?  The flags will
still get set.

Most gpio users, including leds-gpio, can handle gpios being busy.  If
leds-gpio can't get one of the gpios, it rolls back all the leds it did
create, doesn't drive the device and returns EBUSY.  Except with
of_get_gpio() setting the flags, it will change the flags out from under
whatever had the gpio already allocated!
--
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/