Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     syzkaller-bugs@googlegroups.com, Tony Lu <tonylu@linux.alibaba.com>
Cc:     jgg@ziepe.ca, liangwenpeng@huawei.com,
        linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
        liweihang@huawei.com, netdev@vger.kernel.org,
        syzkaller-bugs@googlegroups.com,
        syzbot <syzbot+4f322a6d84e991c38775@syzkaller.appspotmail.com>
Subject: Re: [syzbot] BUG: sleeping function called from invalid context in smc_pnet_apply_ib
Date:   Mon, 21 Feb 2022 11:21:29 +0100
Message-ID: <21388493.EfDdHjke4D@leap>
In-Reply-To: <YhNZAyoqSzIAfF9Y@TonyMac-Alibaba>
References: <000000000000b772b805d8396f14@google.com> <2691692.BEx9A2HvPv@leap> <YhNZAyoqSzIAfF9Y@TonyMac-Alibaba>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On luned=C3=AC 21 febbraio 2022 10:18:59 CET Tony Lu wrote:
> On Thu, Feb 17, 2022 at 07:05:31PM +0100, Fabio M. De Francesco wrote:
> > On gioved=C3=AC 17 febbraio 2022 17:41:22 CET syzbot wrote:
> > > Hello,
> > >=20
> > > syzbot found the following issue on:
> > >=20
> > > HEAD commit:    c832962ac972 net: bridge: multicast: notify switchdev=
 driv..
> > > git tree:       net
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=3D16b157bc7=
00000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=3D266de9da7=
5c71a45
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=3D4f322a6d84e=
991c38775
> > > compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Bi=
nutils for Debian) 2.35.2
> > >=20
> > > Unfortunately, I don't have any reproducer for this issue yet.
> > >=20
> > > IMPORTANT: if you fix the issue, please add the following tag to the =
commit:
> > > Reported-by: syzbot+4f322a6d84e991c38775@syzkaller.appspotmail.com
> > >=20
> > > infiniband syz1: set down
> > > infiniband syz1: added lo
> > > RDS/IB: syz1: added
> > > smc: adding ib device syz1 with port count 1
> > > BUG: sleeping function called from invalid context at kernel/locking/=
mutex.c:577
> > > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 17974, name: s=
yz-executor.3
> > > preempt_count: 1, expected: 0
> > > RCU nest depth: 0, expected: 0
> > > 6 locks held by syz-executor.3/17974:
> > >  #0: ffffffff90865838 (&rdma_nl_types[idx].sem){.+.+}-{3:3}, at: rdma=
_nl_rcv_msg+0x161/0x690 drivers/infiniband/core/netlink.c:164
> > >  #1: ffffffff8d04edf0 (link_ops_rwsem){++++}-{3:3}, at: nldev_newlink=
+0x25d/0x560 drivers/infiniband/core/nldev.c:1707
> > >  #2: ffffffff8d03e650 (devices_rwsem){++++}-{3:3}, at: enable_device_=
and_get+0xfc/0x3b0 drivers/infiniband/core/device.c:1321
> > >  #3: ffffffff8d03e510 (clients_rwsem){++++}-{3:3}, at: enable_device_=
and_get+0x15b/0x3b0 drivers/infiniband/core/device.c:1329
> > >  #4: ffff8880482c85c0 (&device->client_data_rwsem){++++}-{3:3}, at: a=
dd_client_context+0x3d0/0x5e0 drivers/infiniband/core/device.c:718
> > >  #5: ffff8880230a4118 (&pnettable->lock){++++}-{2:2}, at: smc_pnetid_=
by_table_ib+0x18c/0x470 net/smc/smc_pnet.c:1159
> > > Preemption disabled at:
> > > [<0000000000000000>] 0x0
> > > CPU: 1 PID: 17974 Comm: syz-executor.3 Not tainted 5.17.0-rc3-syzkall=
er-00170-gc832962ac972 #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BI=
OS Google 01/01/2011
> > > Call Trace:
> > >  <TASK>
> > >  __dump_stack lib/dump_stack.c:88 [inline]
> > >  dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> > >  __might_resched.cold+0x222/0x26b kernel/sched/core.c:9576
> > >  __mutex_lock_common kernel/locking/mutex.c:577 [inline]
> > >  __mutex_lock+0x9f/0x12f0 kernel/locking/mutex.c:733
> > >  smc_pnet_apply_ib+0x28/0x160 net/smc/smc_pnet.c:251
> > >  smc_pnetid_by_table_ib+0x2ae/0x470 net/smc/smc_pnet.c:1164
> >=20
> > If I recall it well, read_lock() disables preemption.=20
> >=20
> > smc_pnetid_by_table_ib() uses read_lock() and then it calls smc_pnet_ap=
ply_ib()=20
> > which, in turn, calls mutex_lock(&smc_ib_devices.mutex). Therefore the =
code=20
> > acquires a mutex while in atomic and we get a SAC bug.
> >=20
> > Actually, even if my argument is correct(?), I don't know if the read_l=
ock()=20
> > in smc_pnetid_by_table_ib() can be converted to a sleeping lock like a =
mutex or=20
> > a semaphore.
> =20
> Take the email above. I think it is safe to convert read_lock() to
> mutex, which is already used by smc_ib_devices.mutex.

Thanks for your reply.

I have noticed that the "pnettable->lock" rwlock is acquired several times
in different functions of net/smc/smc_pnet.c. smc_pnetid_by_table_ib() is j=
ust one
of many functions that acquire that rwlock.

Therefore, my question is... are you _really_ sure that "pnettable->lock" c=
an be=20
safely converted to a mutex everywhere in net/smc?

I haven't read _all_ the path that lead to {write,read}_lock(&pnettable->lo=
ck) in=20
the net/smc code.

I think that before submitting that patch I should carefully read the code =
and check
_all_ the paths, unless you can confirm that the conversion is safe everywh=
ere. If=20
you can answer my question, I can work on a patch by this evening (CET time=
 zone)=20
and, obviously, give you proper credit.

Thank you,

=46abio M. De Francesco

>=20
> Thank you,
> Tony Lu