On Sat, Oct 5, 2024 at 12:25 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote: > > On Sat, Oct 5, 2024 at 6:54 AM Daniel Yang <danielyangkang@xxxxxxxxx> wrote: > > > > Fixes deadlock described in this bug: > > https://syzkaller.appspot.com/bug?extid=e953a8f3071f5c0a28fd. > > Specific crash report here: > > https://syzkaller.appspot.com/text?tag=CrashReport&x=14670e07980000. > > > > This bug is a false positive lockdep warning since gtp and smc use > > completely different socket protocols. > > > > Lockdep thinks that lock_sock() in smc will deadlock with gtp's > > lock_sock() acquisition. Adding a function that initializes lockdep > > labels for smc socks resolved the false positives in lockdep upon > > testing. Since smc uses AF_SMC and SOCKSTREAM, two labels are created to > > distinguish between proper smc socks and non smc socks incorrectly > > input into the function. > > > > Signed-off-by: Daniel Yang <danielyangkang@xxxxxxxxx> > > Reported-by: syzbot+e953a8f3071f5c0a28fd@xxxxxxxxxxxxxxxxxxxxxxxxx > > --- > > v1->v2: Add lockdep annotations instead of changing locking order > > net/smc/af_smc.c | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > > index 0316217b7..4de70bfd5 100644 > > --- a/net/smc/af_smc.c > > +++ b/net/smc/af_smc.c > > @@ -16,6 +16,8 @@ > > * based on prototype from Frank Blaschka > > */ > > > > +#include "linux/lockdep_types.h" > > +#include "linux/socket.h" > > #define KMSG_COMPONENT "smc" > > #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt > > > > @@ -2755,6 +2757,24 @@ int smc_getname(struct socket *sock, struct sockaddr *addr, > > return smc->clcsock->ops->getname(smc->clcsock, addr, peer); > > } > > > > +static struct lock_class_key smc_slock_key[2]; > > +static struct lock_class_key smc_key[2]; > > + > > +static inline void smc_sock_lock_init(struct sock *sk) > > +{ > > + bool is_smc = (sk->sk_family == AF_SMC) && sk_is_tcp(sk); > > + > > + sock_lock_init_class_and_name(sk, > > + is_smc ? > > + "smc_lock-AF_SMC_SOCKSTREAM" : > > + "smc_lock-INVALID", > > + &smc_slock_key[is_smc], > > + is_smc ? > > + "smc_sk_lock-AF_SMC_SOCKSTREAM" : > > + "smc_sk_lock-INVALID", > > + &smc_key[is_smc]); > > +} > > + > > int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) > > { > > struct sock *sk = sock->sk; > > @@ -2762,6 +2782,7 @@ int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) > > int rc; > > > > smc = smc_sk(sk); > > + smc_sock_lock_init(sk); > > lock_sock(sk); > > > > /* SMC does not support connect with fastopen */ > > -- > > 2.39.2 > > > > sock_lock_init_class_and_name() is not meant to be repeatedly called, > from sendmsg() > > Find a way to do this once, perhaps in smc_create_clcsk(), but I will > let SMC experts chime in. So I tried putting the lockdep annotations in smc_create_clcsk() as well as smc_sock_alloc() and they both fail to remove the false positive but putting the annotations in smc_sendmsg() gets rid of them. I put some print statements in the functions to see the addresses of the socks. [ 78.121827][ T8326] smc: smc_create_clcsk clcsk_addr: ffffc90007f0fd20 [ 78.122436][ T8326] smc: sendmsg sk_addr: ffffc90007f0fa88 [ 78.126907][ T8326] smc: __smc_create input_param clcsock: 0000000000000000 [ 78.134395][ T8326] smc: smc_sock_alloc sk_addr: ffffc90007f0fd70 [ 78.136679][ T8326] smc: smc_create_clcsk clcsk_clcsk: ffffc90007f0fd70 It appears that none of the smc allocation methods are called, so where else exactly could the sock used in sendmsg be created?
