linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH 10/22] gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH
Date: Thu, 25 Jun 2020 17:13:07 +0800	[thread overview]
Message-ID: <20200625091307.GA16386@sol> (raw)
In-Reply-To: <CAHp75VdCGpvoK8RZGwbehOd3eORE+qwFR31ucFxtU4vdc5pvYg@mail.gmail.com>

On Thu, Jun 25, 2020 at 11:44:21AM +0300, Andy Shevchenko wrote:
> On Thu, Jun 25, 2020 at 1:58 AM Kent Gibson <warthog618@gmail.com> wrote:
> > On Wed, Jun 24, 2020 at 11:57:14PM +0800, Kent Gibson wrote:
> > > On Wed, Jun 24, 2020 at 05:46:33PM +0300, Andy Shevchenko wrote:
> > > > On Tue, Jun 23, 2020 at 7:03 AM Kent Gibson <warthog618@gmail.com> wrote:
> 
> ...
> 
> > > > I stumbled over this myself, but...
> > > >
> > > > > -               if (test_bit(hwgpio, gcdev->watched_lines))
> > > > > +               if (test_and_set_bit(hwgpio, gcdev->watched_lines))
> > > > >                         return -EBUSY;
> > > > >
> > > > >                 gpio_desc_to_lineinfo(desc, &lineinfo);
> > > > > @@ -897,7 +897,6 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > > > >                 if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
> > > > >                         return -EFAULT;
> > > > >
> > > > > -               set_bit(hwgpio, gcdev->watched_lines);
> > > > >                 return 0;
> > > >
> > > > ...I think it's not an equivalent despite races involved. If you set
> > > > bit and return error code, you will have the wrong state.
> 
> > Perhaps you are referring to the case where the copy_to_user fails?
> 
> Yes.
> 
> > To be honest I considered that to be so unlikely that I ignored it.
> > Is there a relevant failure mode that I'm missing?
> 
> The traditional question for such cases is "what can possibly go wrong?"
> I wouldn't underestimate the probability of failure.
> 

The worst case is the watch is enabled and the userspace gets an
EFAULT so it thinks it failed.  If userspace retries then they get
EBUSY, so userspace accounting gets muddled.

We can clear the watch bit if the copy_to_user fails - before
returning the EFAULT. Would that be satisfactory?

Back to the failure, is it possible for the copy_to_user fail here,
given that the corresponding copy_from_user has succeeded?
If so, can that be manually triggered for test purposes?

Cheers,
Kent.

  reply	other threads:[~2020-06-25  9:13 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23  4:00 [PATCH 00/22] gpio: cdev: add uAPI V2 Kent Gibson
2020-06-23  4:00 ` [PATCH 01/22] gpiolib: move gpiolib-sysfs function declarations into their own header Kent Gibson
2020-06-24 13:34   ` Bartosz Golaszewski
2020-06-24 13:38     ` Kent Gibson
2020-06-23  4:00 ` [PATCH 02/22] gpiolib: cdev: sort includes Kent Gibson
2020-06-24 13:34   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 03/22] gpiolib: cdev: minor indentation fixes Kent Gibson
2020-06-24 13:35   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 04/22] gpiolib: cdev: refactor gpiohandle_flags_to_desc_flags Kent Gibson
2020-06-24 13:51   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 05/22] gpiolib: cdev: rename 'filep' and 'filp' to 'file' to be consistent with other use Kent Gibson
2020-06-24 13:52   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 06/22] gpiolib: cdev: rename numdescs to num_descs Kent Gibson
2020-06-24 13:53   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 07/22] gpiolib: cdev: remove pointless decrement of i Kent Gibson
2020-06-24 13:53   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 08/22] gpiolib: cdev: complete the irq/thread timestamp handshake Kent Gibson
2020-06-24 14:00   ` Bartosz Golaszewski
2020-06-24 14:08     ` Kent Gibson
2020-06-25  9:44       ` Bartosz Golaszewski
2020-06-25 10:01         ` Kent Gibson
2020-06-30  9:08           ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 09/22] gpiolib: cdev: rename priv to gcdev Kent Gibson
2020-06-24 14:04   ` Bartosz Golaszewski
2020-06-24 14:19     ` Kent Gibson
2020-06-24 14:20       ` Bartosz Golaszewski
2020-06-24 23:16         ` Kent Gibson
2020-06-25 10:26           ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 10/22] gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH Kent Gibson
2020-06-24 14:05   ` Bartosz Golaszewski
2020-06-24 14:46   ` Andy Shevchenko
2020-06-24 15:57     ` Kent Gibson
2020-06-24 22:58       ` Kent Gibson
2020-06-25  8:44         ` Andy Shevchenko
2020-06-25  9:13           ` Kent Gibson [this message]
2020-06-25  9:23             ` Andy Shevchenko
2020-06-25  9:36               ` Kent Gibson
2020-06-23  4:00 ` [PATCH 11/22] gpiolib: cdev: remove recalculation of offset Kent Gibson
2020-06-30  8:56   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 12/22] gpio: uapi: define GPIO_MAX_NAME_SIZE for array sizes Kent Gibson
2020-06-24 14:13   ` Bartosz Golaszewski
2020-06-23  4:00 ` [PATCH 13/22] gpio: uapi: define uAPI V2 Kent Gibson
2020-06-24 14:33   ` Andy Shevchenko
2020-06-24 15:40     ` Kent Gibson
2020-06-26 14:02       ` Kent Gibson
2020-06-24 14:36   ` Bartosz Golaszewski
2020-06-24 23:58     ` Kent Gibson
2020-06-23  4:00 ` [PATCH 14/22] gpiolib: make cdev a build option Kent Gibson
2020-06-29 14:25   ` Bartosz Golaszewski
2020-06-23  4:01 ` [PATCH 15/22] gpiolib: add build option for CDEV V1 ABI Kent Gibson
2020-06-29 14:26   ` Bartosz Golaszewski
2020-06-23  4:01 ` [PATCH 16/22] gpiolib: cdev: add V2 uAPI implementation to parity with V1 Kent Gibson
2020-06-23 17:44   ` Dan Carpenter
2020-06-23 23:23     ` Kent Gibson
2020-06-23  4:01 ` [PATCH 17/22] gpiolib: cdev: report edge detection in lineinfo Kent Gibson
2020-06-23  4:01 ` [PATCH 18/22] gpiolib: cdev: support setting debounce Kent Gibson
2020-06-23  4:01 ` [PATCH 19/22] gpio: uapi: document uAPI V1 as deprecated Kent Gibson
2020-06-23  4:01 ` [PATCH 20/22] tools: gpio: switch tools to V2 uAPI Kent Gibson
2020-06-23  4:01 ` [PATCH 21/22] tools: gpio: add debounce support to gpio-event-mon Kent Gibson
2020-06-23  4:01 ` [PATCH 22/22] tools: gpio: support monitoring multiple lines Kent Gibson
2020-06-30 10:43   ` Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200625091307.GA16386@sol \
    --to=warthog618@gmail.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).