| Message ID | 20240628-dbus-v2-10-c1331ac17cb8@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dbus: add GLib-based DBus daemon and command-line client | expand |
On Friday 28 June 2024 11:58:29 GMT-7 Bartosz Golaszewski wrote: > + <!-- > + Used: > + > + True if line is busy. > + > + Line can be used by gpio-manager, another user-space process, a > kernel + driver or is hogged. The exact reason a line is busy cannot > be determined + from user-space unless it's known to be managed by > gpio-manager (see: + the Managed property of this interface). > + --> > + <property name='Used' type='b' access='read'/> What's the point of this property? It looks racy, as the user (whichever it is) can stop using it soon after a true read, or the line can become used right after a false read? The latter could lead to TOCTOU problems. Wouldn't it be better to force users to RequestLine and get an error if the line is busy? Because if it wasn't busy, now the calling application knows nothing else can grab it. Speaking of, RequestLine should document the errors it may return.
Hi Thiago, On Sun, 2024-06-30 at 09:49 -0700, Thiago Macieira wrote: > > + True if line is busy. > > + > > + Line can be used by gpio-manager, another user-space process, a > > kernel + driver or is hogged. The exact reason a line is busy cannot > > be determined + from user-space unless it's known to be managed by > > gpio-manager (see: + the Managed property of this interface). > > + --> > > + <property name='Used' type='b' access='read'/> > > What's the point of this property? It looks racy, as the user (whichever it > is) can stop using it soon after a true read, or the line can become used > right after a false read? The latter could lead to TOCTOU problems. > > Wouldn't it be better to force users to RequestLine and get an error if the > line is busy? Because if it wasn't busy, now the calling application knows > nothing else can grab it. this approach would make the inspection itself racy, isn't it? I'm thinking about two instances of "gpiocli info" running in parallel, they would display GPIO lines randomly "busy" even in case none of them actually is?
On Tuesday 2 July 2024 08:48:43 CEST Sverdlin, Alexander wrote: > > What's the point of this property? It looks racy, as the user (whichever > > it > > is) can stop using it soon after a true read, or the line can become used > > right after a false read? The latter could lead to TOCTOU problems. > > > > Wouldn't it be better to force users to RequestLine and get an error if > > the > > line is busy? Because if it wasn't busy, now the calling application knows > > nothing else can grab it. > > this approach would make the inspection itself racy, isn't it? > I'm thinking about two instances of "gpiocli info" running in parallel, they > would display GPIO lines randomly "busy" even in case none of them actually > is? Correct, but the race time would be very small. The status application need not keep the line requested for long, only enough to get the current state. In any case, my argument is for everything *except* for the status / info application. That's an outlier application, of which there's likely to be a single one in the system. However, there will likely be multiple applications that need lines for actual uses. The argument is that the presence of the property can lead application authors to check before RequestLine in order to present a message to their users, possibly because their code is simpler for reading a property than dealing with an error. Therefore, my advice is to not have the API that can lead to TOCTOU, even if by accident.
On Tue, Jul 2, 2024 at 11:06 AM Thiago Macieira <thiago@kde.org> wrote: > > On Tuesday 2 July 2024 08:48:43 CEST Sverdlin, Alexander wrote: > > > What's the point of this property? It looks racy, as the user (whichever > > > it > > > is) can stop using it soon after a true read, or the line can become used > > > right after a false read? The latter could lead to TOCTOU problems. > > > > > > Wouldn't it be better to force users to RequestLine and get an error if > > > the > > > line is busy? Because if it wasn't busy, now the calling application knows > > > nothing else can grab it. > > > > this approach would make the inspection itself racy, isn't it? > > I'm thinking about two instances of "gpiocli info" running in parallel, they > > would display GPIO lines randomly "busy" even in case none of them actually > > is? > > Correct, but the race time would be very small. The status application need > not keep the line requested for long, only enough to get the current state. > > In any case, my argument is for everything *except* for the status / info > application. That's an outlier application, of which there's likely to be a > single one in the system. However, there will likely be multiple applications > that need lines for actual uses. The argument is that the presence of the > property can lead application authors to check before RequestLine in order to > present a message to their users, possibly because their code is simpler for > reading a property than dealing with an error. > > Therefore, my advice is to not have the API that can lead to TOCTOU, even if > by accident. Unfortunately there's one issue with the above: requesting a line (even as input) may result in the kernel driver triggering a physical change in hardware which may be undesirable. Inspecting the "Used" property only results in fetching a flag from the kernel and will never make the driver act upon HW. Bartosz
Hi Bartosz, Thiago! On Tue, 2024-07-02 at 11:15 +0200, Bartosz Golaszewski wrote: > > > > What's the point of this property? It looks racy, as the user (whichever > > > > it > > > > is) can stop using it soon after a true read, or the line can become used > > > > right after a false read? The latter could lead to TOCTOU problems. Actually I don't think those users who are going to request the line will look into "busy" flag, they just need to request directly and [possibly] fail. But for displaying the status the racy flag is perfectly fine, because the information is anyway outdated by the time it appears on the terminal. > > > > Wouldn't it be better to force users to RequestLine and get an error if > > > > the > > > > line is busy? Because if it wasn't busy, now the calling application knows > > > > nothing else can grab it. > > > > > > this approach would make the inspection itself racy, isn't it? > > > I'm thinking about two instances of "gpiocli info" running in parallel, they > > > would display GPIO lines randomly "busy" even in case none of them actually > > > is? > > > > Correct, but the race time would be very small. The status application need > > not keep the line requested for long, only enough to get the current state. > > > > In any case, my argument is for everything *except* for the status / info > > application. That's an outlier application, of which there's likely to be a > > single one in the system. However, there will likely be multiple applications > > that need lines for actual uses. The argument is that the presence of the > > property can lead application authors to check before RequestLine in order to > > present a message to their users, possibly because their code is simpler for > > reading a property than dealing with an error. > > > > Therefore, my advice is to not have the API that can lead to TOCTOU, even if > > by accident. > > Unfortunately there's one issue with the above: requesting a line > (even as input) may result in the kernel driver triggering a physical > change in hardware which may be undesirable. Inspecting the "Used" > property only results in fetching a flag from the kernel and will > never make the driver act upon HW. And this is very true, barely anyone wants their "input pull-up" or "input pull-down" to become simply "input" just by observing the status.
On Tuesday 2 July 2024 11:15:11 CEST Bartosz Golaszewski wrote: > Therefore, my advice is to not have the API that can lead to TOCTOU, even > if > > by accident. > > Unfortunately there's one issue with the above: requesting a line > (even as input) may result in the kernel driver triggering a physical > change in hardware which may be undesirable. Inspecting the "Used" > property only results in fetching a flag from the kernel and will > never make the driver act upon HW. Ah, that makes sense. Thank you for the explanation.
diff --git a/dbus/lib/io.gpiod1.xml b/dbus/lib/io.gpiod1.xml new file mode 100644 index 0000000..ca95302 --- /dev/null +++ b/dbus/lib/io.gpiod1.xml @@ -0,0 +1,318 @@ +<!-- SPDX-License-Identifier: CC-BY-SA-4.0 --> +<!-- SPDX-FileCopyrightText: 2022-2024 Bartosz Golaszewski <bartosz.golaszewski@linaro.org> --> + +<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" + "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd"> + +<node> + + <!-- + io.gpiod1.Chip: + @short_description: Represents a single GPIO chip in the system. + --> + <interface name='io.gpiod1.Chip'> + + <!-- + Name: + + Name of the chip as represented in the kernel. + --> + <property name='Name' type='s' access='read'/> + + <!-- + Label: + + Label of the chip as represented in the kernel. + --> + <property name='Label' type='s' access='read'/> + + <!-- + NumLines: + + Number of GPIO lines exposed by this chip. + --> + <property name='NumLines' type='u' access='read'/> + + <!-- + Path: + + Filesystem path used to open this chip. + --> + <property name='Path' type='s' access='read'/> + + <!-- + RequestLines: + @line_config: Line configuration. See below for details. + @request_config: Request configuration. See below for details. + @request_path: Object path pointing to the newly added request. + + Requests a set of lines and makes it possible for the users of this API + to manipulate them depending on the line configuration. + + Line configuration is a tuple of two arrays. The first one contains + mappings of arrays of line offsets to sets of line settings. The second + contains the list of default output values which are only used in output + mode. + + Available line config options: + + "direction" => String representing the line direction. Accepts the + following values: "input", "output". + "edge" => String representing the edge detection setting. Accepts the + following values: "falling", "rising", "both". + "active-low" => Boolean representing the active-low setting. + "drive" => String representing the drive settings. Accepts the + following values: "push-pull", "open-drain", "open-source". + "bias" => String representing the internal bias settings. Accepts the + following values: "disabled", "pull-up", "pull-down", "as-is". + "debounce-period" => Debounce period in milliseconds represented as a + signed, 64-bit integer. + "event-clock" => String representing the clock used to timestamp edge + events. Accepts the following values: "monotonic", + "realtime", "hte". + + Output values are applied to the lines in the order they appear in the + settings mappings. + + Example variant that allows to request lines at offsets 1, 5 and 11 in + output, push-pull and active-low modes and specifies the output values + as active (as visualized with g_variant_print()): + + // Line config tuple + ( + // Array of line settings mappings + [ + // Single mapping tuple + ( + // Offsets to map + [1, 5, 11], + // Line settings dict + { + 'direction': <'output'>, + 'drive': <'push-pull'>, + 'active-low': <true> + } + ) + ], + // Output values + [1, 1, 1] + ) + + Request configuration is a hashmap mapping names of the available config + options to their values wrapped in a variant. + + Available request config options: + + "consumer" => Consumer name as a string + "event-buffer-size" => Requested size of the in-kernel edge event + buffer as an unsigned 32-bit integer. + + The object path to the new request is returned on success. The user + should wait for it to appear before trying to use the requested lines in + any way. + --> + <method name='RequestLines'> + <arg name='line_config' direction='in' type='(a(aua{sv})ai)'/> + <arg name='request_config' direction='in' type='a{sv}'/> + <arg name='request_path' direction='out' type='o'/> + </method> + + </interface> + + <!-- + io.gpiod1.Line: + @short_description: Represents a single GPIO line on a chip. + --> + <interface name='io.gpiod1.Line'> + + <!-- + Offset: + + Uniquely identifies the line on the chip. + --> + <property name='Offset' type='u' access='read'/> + + <!-- + Name: + + Name of the GPIO line as represented in the kernel. + --> + <property name='Name' type='s' access='read'/> + + <!-- + Used: + + True if line is busy. + + Line can be used by gpio-manager, another user-space process, a kernel + driver or is hogged. The exact reason a line is busy cannot be determined + from user-space unless it's known to be managed by gpio-manager (see: + the Managed property of this interface). + --> + <property name='Used' type='b' access='read'/> + + <!-- + Consumer: + + Name of the consumer of the line. + --> + <property name='Consumer' type='s' access='read'/> + + <!-- + Direction: + + Direction of the line. Returns "input" or "output". + --> + <property name='Direction' type='s' access='read'/> + + <!-- + EdgeDetection: + + Edge detection settings of the line. Returns: "none", "falling", + "rising" or "both". + --> + <property name='EdgeDetection' type='s' access='read'/> + + <!-- + Bias: + + Bias setting of the line. Returns: "unknown", "disabled, "pull-up" or + "pull-down". + --> + <property name='Bias' type='s' access='read'/> + + <!-- + Drive: + + Drive setting of the line. Returns "push-pull", "open-source" or + "open-drain". + --> + <property name='Drive' type='s' access='read'/> + + <!-- + ActiveLow: + + True if the line is active-low. False for active-high. + --> + <property name='ActiveLow' type='b' access='read'/> + + <!-- + Debounced: + + True if line is being debounced on interrupts. Can only be true with + edge-detection enabled. + --> + <property name='Debounced' type='b' access='read'/> + + <!-- + DebouncePeriodUs: + + Debounce period in milliseconds. 0 if the line is not debounced. Can + only be non-zero with edge-detection enabled. + --> + <property name='DebouncePeriodUs' type='t' access='read'/> + + <!-- + EventClock: + + System clock used to timestamp edge events on this line. Returns: + "monotonic", "realtime" or "hte". + --> + <property name='EventClock' type='s' access='read'/> + + <!-- + Managed: + + True if the line is managed by gpio-manager. + --> + <property name='Managed' type='b' access='read'/> + + <!-- + RequestPath: + + If this line is managed by gpio-manager then this property will contain + the DBus object path pointing to the managing request object. + --> + <property name='RequestPath' type='o' access='read'/> + + <!-- + EdgeEvent: + @event_data: Contains the edge (1 for rising, 0 for falling), timestamp + in milliseconds and the global & line-local sequence numbers. + + If the line is managed by the gpio-manager and is requested with edge + detection enabled then this signal will be emitted for every edge event + registered on this line. + --> + <signal name='EdgeEvent'> + <arg name='event_data' type='(ittt)'/> + </signal> + + </interface> + + <!-- + io.gpiod1.Request: + @short_description: Represents a set of requested GPIO lines. + --> + <interface name='io.gpiod1.Request'> + + <!-- + ChipPath: + + DBus object path pointing to the chip exposing the lines held by this + request. + --> + <property name='ChipPath' type='o' access='read'/> + + <!-- + LinePaths: + + Array of DBus object paths pointing to the lines held by this request. + --> + <property name='LinePaths' type='ao' access='read'/> + + <!-- + Release: + + Release the requested lines. After this method returns successfully, the + request object on which it was called will be destroyed. + --> + <method name='Release'/> + + <!-- + ReconfigureLines: + @line_config: Line configuration. Refer to the RequestLines method of + the io.gpiod1.Chip interface for details. + + Change the configuration of lines held by this request object without + releasing them. + --> + <method name='ReconfigureLines'> + <arg name='line_config' direction='in' type='(a(aua{sv})ai)'/> + </method> + + <!-- + GetValues: + @offsets: Array of line offsets within the request to read values for. + @values: Array of values in the order lines were specified in @offsets. + + Read the values for a set of lines held by the request. + --> + <method name='GetValues'> + <arg name='offsets' direction='in' type='au'/> + <arg name='values' direction='out' type='ai'/> + </method> + + <!-- + SetValues: + @values: Array of mappings from line offsets to desired output values. + + Set the values for a set of lines held by the request. + --> + <method name='SetValues'> + <arg name='values' direction='in' type='a{ui}'/> + </method> + + </interface> + +</node>