mbox series

[libgpiod,0/2] fix potential discarding of events by read events

Message ID 20200912022248.16240-1-warthog618@gmail.com
Headers show
Series fix potential discarding of events by read events | expand

Message

Kent Gibson Sept. 12, 2020, 2:22 a.m. UTC 
This patch set addresses a bug where reading events can quietly discard
events.  The problem occurs any time a request made is for a number of
events that differs from the number available at the time - a number
that userspace is unaware of.

The problem is due to the read multiple always reading up to 16 events
from the kernel - independent of how many events have been requested.

The problem applies to reading both single and multiple events, as the
single event read implementation wraps the multiple.

The first patch extends test coverage to highlight the problem.
The second fixes the bug.

Kent Gibson (2):
  tests: event: reading test coverage extended to cover reading a subset
    of available events
  core: fix reading subset of available events

 lib/core.c          |   5 +-
 tests/tests-event.c | 144 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 128 insertions(+), 21 deletions(-)


base-commit: fc61e740fcbe3c6594295766759888c96c45bd29

Comments

Kent Gibson Sept. 12, 2020, 3:54 a.m. UTC | #1 
On Sat, Sep 12, 2020 at 10:22:47AM +0800, Kent Gibson wrote:
> Add tests for gpiod_line_event_read(), including reading multiple
> entries from the kernel event kfifo, and extend the existing
> read_multiple_event tests to read a subset of the available events as
> well as all the available events.
> 
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
> ---
> 
> I have removed the usleep()s between setting the pulls on the mockup as
> I don't believe they are necessary.
> Setting the pulls results in serial writes to the debugfs, which the
> kernel should be able to deal with.
> Any queuing in the kernel that results should come out in the wash.
> 

I'm obviously missing something in the path from gpio-mockup to
gpiolib, as the tests fail in slower emulated environments :-(.
I'll put out a v2 with the usleep()s restored once I finish testing
in a more diverse set of environments.

Sorry for rushing this one out.

Cheers,
Kent.