Re: [PATCH 3/3] exec: Transform exec_update_mutex into a rw_semaphore

[Linus Torvalds]

On Fri, Dec 4, 2020 at 8:08 AM Bernd Edlinger <bernd.edlinger@xxxxxxxxxx> wrote:
>
> >
> > -static void kcmp_unlock(struct mutex *m1, struct mutex *m2)
> > +static void kcmp_unlock(struct rw_semaphore *l1, struct rw_semaphore *l2)
> >  {
> > -     if (likely(m2 != m1))
> > -             mutex_unlock(m2);
> > -     mutex_unlock(m1);
> > +     if (likely(l2 != l1))
>
> is this still necessary ?
>
> > +             up_read(l2);
> > +     up_read(l1);
> >  }
> >
> > -static int kcmp_lock(struct mutex *m1, struct mutex *m2)
> > +static int kcmp_lock(struct rw_semaphore *l1, struct rw_semaphore *l2)
> >  {
> >       int err;
> >
> > -     if (m2 > m1)
> > -             swap(m1, m2);
> > +     if (l2 > l1)
> > +             swap(l1, l2);
>
> and this is probably also no longer necessary?

These are still necessary, because even a recursive read lock can
still block on a writer trying to come in between the two read locks
due to fairness guarantees.

So taking the same read lock twice is still a source of possible deadlocks.

For the same reason, read locks still have ABBA deadlock and need to
be taken in order.

So switching from a mutex to a rwlock doesn't really change the
locking rules in this respect.

In fact, I'm not convinced this change even fixes the deadlock that
syzbot reported, for the same reason: it just requires a write lock in
between two read locks to deadlock.

               Linus