| Message ID | 20200912081105.1615496-3-warthog618@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix potential discarding of events by read events | expand |
On Sat, Sep 12, 2020 at 10:11 AM Kent Gibson <warthog618@gmail.com> wrote: > > Only read the requested number of events from the kernel rather than > reading up to 16 and quietly discarding any surplus. > > The previous behavour is particularly bad for reading single events as > userspace must read the events as quickly as they arrive, effectively > negating the presence of the kernel event kfifo. > > Fixes: 44921ecc9a00 (core: provide functions for reading multiple > line events at once) > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > --- > lib/core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/core.c b/lib/core.c > index ad76051..b964272 100644 > --- a/lib/core.c > +++ b/lib/core.c > @@ -1090,7 +1090,10 @@ int gpiod_line_event_read_fd_multiple(int fd, struct gpiod_line_event *events, > > memset(evdata, 0, sizeof(evdata)); > > - rd = read(fd, evdata, sizeof(evdata)); > + if (num_events > 16) > + num_events = 16; > + > + rd = read(fd, evdata, num_events * sizeof(*evdata)); > if (rd < 0) { > return -1; > } else if ((unsigned int)rd < sizeof(*evdata)) { > -- > 2.28.0 > Wow this is a bad one, thanks for catching this! Will apply shortly and backport it to stable branches too. Thanks! Bartosz
On Mon, Sep 14, 2020 at 11:16 AM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > On Sat, Sep 12, 2020 at 10:11 AM Kent Gibson <warthog618@gmail.com> wrote: > > Fixes: 44921ecc9a00 (core: provide functions for reading multiple > > line events at once) > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > Will apply shortly and backport it to stable branches too. Please note that - Fixes (or actually any tag) should be one per line - no blank lines in the tag block
On Mon, Sep 14, 2020 at 5:16 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 14, 2020 at 11:16 AM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > On Sat, Sep 12, 2020 at 10:11 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > Fixes: 44921ecc9a00 (core: provide functions for reading multiple > > > line events at once) > > > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > > > > Will apply shortly and backport it to stable branches too. > > Please note that > - Fixes (or actually any tag) should be one per line > - no blank lines in the tag block > Kent: no need to resend this, I fixed it when applying, but yeah for the future it's true. Bart
On Mon, Sep 14, 2020 at 6:20 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > On Mon, Sep 14, 2020 at 5:16 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Mon, Sep 14, 2020 at 11:16 AM Bartosz Golaszewski > > <bgolaszewski@baylibre.com> wrote: > > > On Sat, Sep 12, 2020 at 10:11 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > Fixes: 44921ecc9a00 (core: provide functions for reading multiple > > > > line events at once) > > > > > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > > > > > > > Will apply shortly and backport it to stable branches too. > > > > Please note that > > - Fixes (or actually any tag) should be one per line > > - no blank lines in the tag block > > > > Kent: no need to resend this, I fixed it when applying, but yeah for > the future it's true. JFYI: I'm about to look at v8 patch 4 of uAPI v2 series.
On Mon, Sep 14, 2020 at 06:16:35PM +0300, Andy Shevchenko wrote: > On Mon, Sep 14, 2020 at 11:16 AM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > On Sat, Sep 12, 2020 at 10:11 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > Fixes: 44921ecc9a00 (core: provide functions for reading multiple > > > line events at once) > > > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > > > > Will apply shortly and backport it to stable branches too. > > Please note that > - Fixes (or actually any tag) should be one per line > - no blank lines in the tag block > Sorry - I wasn't aware that tags are exempt to the line length limits. And checkpatch.pl doesn't pick up those rules either :-(. Cheers, Kent.
diff --git a/lib/core.c b/lib/core.c index ad76051..b964272 100644 --- a/lib/core.c +++ b/lib/core.c @@ -1090,7 +1090,10 @@ int gpiod_line_event_read_fd_multiple(int fd, struct gpiod_line_event *events, memset(evdata, 0, sizeof(evdata)); - rd = read(fd, evdata, sizeof(evdata)); + if (num_events > 16) + num_events = 16; + + rd = read(fd, evdata, num_events * sizeof(*evdata)); if (rd < 0) { return -1; } else if ((unsigned int)rd < sizeof(*evdata)) {
Only read the requested number of events from the kernel rather than reading up to 16 and quietly discarding any surplus. The previous behavour is particularly bad for reading single events as userspace must read the events as quickly as they arrive, effectively negating the presence of the kernel event kfifo. Fixes: 44921ecc9a00 (core: provide functions for reading multiple line events at once) Signed-off-by: Kent Gibson <warthog618@gmail.com> --- lib/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)