Re: [PATCH 10/22] gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH

From: Andy Shevchenko
Date: Thu Jun 25 2020 - 05:24:05 EST


On Thu, Jun 25, 2020 at 12:13 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> 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@xxxxxxxxx> wrote:
> > > On Wed, Jun 24, 2020 at 11:57:14PM +0800, Kent Gibson wrote:

...

> > > 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?

Perhaps. I didn't check that scenario.

> Back to the failure, is it possible for the copy_to_user fail here,
> given that the corresponding copy_from_user has succeeded?

Of course. The general rule is if on SMP system you have not strongly
serialized sequence of calls (means no preemption, no interrupts, etc)
anything can happen in between.

> If so, can that be manually triggered for test purposes?

Unfortunately not an expert in mm, no idea, sorry.

--
With Best Regards,
Andy Shevchenko