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

Linus Torvalds
2020-12-04T17:22:57Z

On Fri, Dec 4, 2020 at 8:08 AM Bernd Edlinger <bernd.edlinger@hotmail.de> 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