On Thu, Jun 25, 2020 at 1:58 AM Kent Gibson <warthog...@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 <warthog...@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. -- With Best Regards, Andy Shevchenko
