netdev - Re: [not-yet-signed PATCH] RDMA/core: flush gid_cache_wq WQ from disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <13f54775-7a36-48f2-b9cd-62ab9f15a82b@I-love.SAKURA.ne.jp>
Date: Tue, 16 Dec 2025 20:28:08 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Leon Romanovsky <leon@...nel.org>, Majd Dibbiny <majd@...lanox.com>,
        Doug Ledford <dledford@...hat.com>, Jason Gunthorpe <jgg@...dia.com>,
        Yuval Shaia <yshaia@...vell.com>
Cc: Bernard Metzler <bernard.metzler@...ux.dev>,
        OFED mailing list <linux-rdma@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [not-yet-signed PATCH] RDMA/core: flush gid_cache_wq WQ from
 disable_device()

Hello, Jason Gunthorpe.

Commit 0df91bb67334 ("RDMA/devices: Use xarray to store the client_data")
says

    Since xarray can store 'going_down' using a mark just entirely eliminate
    the struct ib_client_data and directly store the client_data value in the
    xarray. However this does require a special iterator as we must still
    iterate over any NULL client_data values.

and started using DEVICE_REGISTERED mark. But It seems to me that that commit
has missed that ib_enum_roce_netdev() from ib_enum_all_roce_netdevs() is called
asynchronously from WQ context. Due to that commit, ib_enum_roce_netdev() became
no longer being called as soon as DEVICE_REGISTERED is cleared in
ib_unregister_device(); I guess that that commit needed to wait for pending
work items in gid_cache_wq WQ to complete. What do you think?

On 2025/12/15 23:09, Tetsuo Handa wrote:
> On 2025/12/11 22:24, Tetsuo Handa wrote:
>> Since a reproducer for this bug is not available, I haven't verified
>> whether this is a bug syzbot is currently reporting in
>> https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84 .
>> But I'd like to add Reported-by: syzbot if netdevice_event_work_handler()
>> is supposed to be called for releasing GID entry upon NETDEV_UNREGISTER
>> event. Thus, please review this change.
> 
> I can observe using simple atomic_t counters that there are sometimes pending
> netdevice_event() works as of immediately before clearing DEVICE_REGISTERED flag.
> That is, clearing DEVICE_REGISTERED flag without flushing pending netdevice_event()
> works results in failing to process some of netdev events.
> 
> I considered resolving DEVICE_REGISTERED flag inside netdevice_event() and then
> flush pending netdevice_event() works after clearing DEVICE_REGISTERED flag (diff
> is shown below). But I immediately got circular locking dependency problem by just
> executing "rdma link add siw0 type siw netdev lo" command line. Therefore, I guess
> that the reason RDMA code defers netdevice_event() handling to WQ context is to
> avoid circular locking dependency problem. But I guess that due to lack of reliable
> flushing mechanism when clearing DEVICE_REGISTERED flag, sometimes operations for
> deleting GID entry are not invoked, and syzbot is reporting refcount leak...