On Mon, 21 Jun 2021 19:18:56 +0200 Guvenc Gulce <guvenc@xxxxxxxxxxxxx> wrote: > > > On 21/06/2021 16:56, Pavel Skripkin wrote: > > On Sun, 20 Jun 2021 16:22:16 -0700 > > syzbot <syzbot+5dda108b672b54141857@xxxxxxxxxxxxxxxxxxxxxxxxx> > > wrote: > > > >> Hello, > >> > >> syzbot found the following issue on: > >> > >> HEAD commit: 0c337952 Merge tag > >> 'wireless-drivers-next-2021-06-16' of g.. git tree: net-next > >> console output: > >> https://syzkaller.appspot.com/x/log.txt?x=1621de10300000 kernel > >> config: https://syzkaller.appspot.com/x/.config?x=a6380da8984033f1 > >> dashboard link: > >> https://syzkaller.appspot.com/bug?extid=5dda108b672b54141857 syz > >> repro: > >> https://syzkaller.appspot.com/x/repro.syz?x=121d2d20300000 C > >> reproducer: > >> https://syzkaller.appspot.com/x/repro.c?x=100bd768300000 > >> > >> The issue was bisected to: > >> > >> commit f9006acc8dfe59e25aa75729728ac57a8d84fc32 > >> Author: Florian Westphal <fw@xxxxxxxxx> > >> Date: Wed Apr 21 07:51:08 2021 +0000 > >> > >> netfilter: arp_tables: pass table pointer via nf_hook_ops > >> > > I think, bisection is wrong this time :) > > > > It should be e0e4b8fa533858532f1b9ea9c6a4660d09beb37a ("net/smc: > > Add SMC statistics support") > > > > > > Some debug results: > > > > syzkaller repro just opens the socket and calls sendmsg. Ftrace log: > > > > > > 0) | smc_create() { > > 0) | smc_sock_alloc() { > > 0) + 88.493 us | smc_hash_sk(); > > 0) ! 131.487 us | } > > 0) ! 189.912 us | } > > 0) | smc_sendmsg() { > > 0) 2.808 us | smc_tx_sendmsg(); > > 0) ! 148.484 us | } > > > > > > That means, that smc_buf_create() wasn't called at all, so we need > > to check sndbuf_desc before dereferencing > > > > Something like this should work > > > > diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c > > index 075c4f4b4..e24071b12 100644 > > --- a/net/smc/smc_tx.c > > +++ b/net/smc/smc_tx.c > > @@ -154,7 +154,7 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct > > msghdr *msg, size_t len) goto out_err; > > } > > > > - if (len > conn->sndbuf_desc->len) > > + if (conn->sndbuf_desc && len > conn->sndbuf_desc->len) > > SMC_STAT_RMB_TX_SIZE_SMALL(smc, !conn->lnk); > > > > if (len > conn->peer_rmbe_size) > > > > > > Thoughts? > > > > > > +CC Guvenc Gulce > > > > > > With regards, > > Pavel Skripkin > > Thanks for analyzing the cause. Your approach would work but I would > prefer that we check the state of the socket before doing the > statistics relevant if check. This will ensure that smc_buf_create() > was already called. I am testing the fix at the moment which would > look like the following: > This sounds better. I knew, that my approach will just "silence" the bug, that's why I CCed you for better approach :) > diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c > index 075c4f4b41cf..289025cd545a 100644 > --- a/net/smc/smc_tx.c > +++ b/net/smc/smc_tx.c > @@ -154,6 +154,9 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct > msghdr *msg, size_t len) goto out_err; > } > > + if (sk->sk_state == SMC_INIT) > + return -ENOTCONN; > + > if (len > conn->sndbuf_desc->len) > SMC_STAT_RMB_TX_SIZE_SMALL(smc, !conn->lnk); > > With regards, Pavel Skripkin
