Re: [PATCH] Auxiliary display drivers: fix possible race condition

[Andy Shevchenko]

On Mon, May 11, 2026 at 11:56 PM Alexander A. Klimov
<grandmaster@xxxxxxxxxxxx> wrote:
> On 5/11/26 12:38, Andy Shevchenko wrote:
> > On Sun, May 10, 2026 at 06:06:46PM +0200, Alexander A. Klimov wrote:
> >> Before linedisp_unregister(), call disable_delayed_work_sync(),
> >> not just cancel_delayed_work_sync().
> >>
> >> While cancel_delayed_work_sync() cancels queued work and
> >> awaits running work, it doesn't reject future work, such as
> >> scheduled by the timer which is set up in linedisp_register().
> >> This timer may fire just before linedisp_unregister() stops it
> >> and cause kind of use after free.
> >>
> >> In contrast, disable_delayed_work_sync() also prevents future work.
> >
> > First of all, split on per-driver basis.
> > Second, there are not so many users of this API
> >
> > $ git grep -lw disable_delayed_work_sync | wc
> >       29      29     882
> >
> > $ git grep -lw cancel_delayed_work_sync | wc
> >      826     826   29389
> >
> > Are many of them prone to the same issue? I don't think so. As far as I read
>
> Why not?
> Maybe I just yet had not the time to check all of them?

Maybe, but from my experience I tend to believe that this is just
about misunderstanding the object lifetime ranges in each of the cases
(either when disable_*() is used when cancel_*() is okay and vice
versa).

> > the code the original variant is fine as long as we guarantee that there may
>
> Compared to "fine as long as", wouldn't a simple "fine" be an upgrade?

The same can be applied to the use of atomic versus spin lock. Yes, we
can do spin locks mostly everywhere, with a cost. I expect a deeper
analysis in the commit message.

> > not be any new work scheduling. In this case the scheduling done whenever we
> > update the line on the display. This can be initiated via user space.
> > So the question here is the user space is quiet at that time or not. Seems
>
> But the timer may fire just before linedisp_unregister() stops it.
> It also schedules work.
>
> > not to me. Now, shouldn't just ordering of the unregister and cancellation
> > of the work fix the issue? (It doesn't seem that the works are doing anything
> > with the linedisp device.)
>
> To me it seems the opposite, that's why I didn't just reorder.
> But I may be wrong.

> > Also note there is a bigger issue that had been reported a week or so ago.
> > The problem is that driver removal may lead to UAF. Fixing that problem may
> > help to address this one as well for all of the linedisp users (switching to
>
> So I'll wait.

That one is not in progress, it's just a known issue. In case you want
to help with that, it will be much appreciated.

> > use managed work cancellation that will be done in time, id est after device
> > is gone).

-- 
With Best Regards,
Andy Shevchenko