linux-kernel - Re: [PATCH v3 6/6] gpiolib: notify user-space about in-kernel line state changes

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]

Message-ID: <20241016072730.GA120095@rigel>
Date: Wed, 16 Oct 2024 15:27:30 +0800
From: Kent Gibson <warthog618@...il.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v3 6/6] gpiolib: notify user-space about in-kernel line
 state changes

On Wed, Oct 16, 2024 at 01:19:44PM +0800, Kent Gibson wrote:
> On Tue, Oct 15, 2024 at 12:56:18PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > -	return gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
> > +	ret = gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
> > +	if (ret == 0) {
> > +		/* These are the only options we notify the userspace about. */
> > +		switch (pinconf_to_config_param(config)) {
> > +		case PIN_CONFIG_BIAS_DISABLE:
> > +		case PIN_CONFIG_BIAS_PULL_DOWN:
> > +		case PIN_CONFIG_BIAS_PULL_UP:
> > +		case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> > +		case PIN_CONFIG_DRIVE_OPEN_SOURCE:
> > +		case PIN_CONFIG_DRIVE_PUSH_PULL:
> > +		case PIN_CONFIG_INPUT_DEBOUNCE:
> > +			gpiod_line_state_notify(desc,
> > +						GPIO_V2_LINE_CHANGED_CONFIG);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +
> > +	return ret;
> >  }
>
> Ah, the debounce - I forgot about that, and other features that cdev
> might emulate.
>
> What happens if userspace requests a line with debounce that is
> supported by hardware?  Seems to me we'll see both a LINE_REQUESTED and a
> LINE_CONFIG_CHANGED when the line is requested.
>

This is problematic for me to test at the moment, as gpiosim doesn't support
debounce. Any chance we could make that configurable?  Similarly drive.

> Conversely, what if a config change impacts features that don't result in a
> notification from gpiod_set_config(), like active low, or emulated
> drive or debounce?
>

Bah, drive is emulated in gpiolib itself, so that should be fine.

When changing config cdev always calls gpiod_direction_input/output(), so I
think that covers the active low case.

But I have a test taking a line from input to output|open_drain and I
get two change events.  The first is the most interesting as it reports
input|open_drain, the second then reports output|open_drain.
That is due to gpiod_direction_output() calling gpiod_set_config() to
set the drive, and later to set the direction, in that order.
Given it will be setting the direction, it should inhibit the event from
the drive setting?

Still haven't tested any debounce changes...

Cheers,
Kent.