netdev - Re: [PATCH net 1/3] can: j1939: prevent deadlock by changing j1939_socks

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: <1b9377e954adab3cd007d77f8e618d3d49753f6f.camel@redhat.com>
Date: Thu, 15 Feb 2024 12:43:42 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>, netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, linux-can@...r.kernel.org, 
	kernel@...gutronix.de, Ziqi Zhao <astrajoan@...oo.com>, 
	syzbot+1591462f226d9cbf0564@...kaller.appspotmail.com, Oleksij Rempel
	 <o.rempel@...gutronix.de>, stable@...r.kernel.org
Subject: Re: [PATCH net 1/3] can: j1939: prevent deadlock by changing
 j1939_socks_lock to rwlock

On Wed, 2024-02-14 at 14:59 +0100, Marc Kleine-Budde wrote:
> From: Ziqi Zhao <astrajoan@...oo.com>
> 
> The following 3 locks would race against each other, causing the
> deadlock situation in the Syzbot bug report:
> 
> - j1939_socks_lock
> - active_session_list_lock
> - sk_session_queue_lock
> 
> A reasonable fix is to change j1939_socks_lock to an rwlock, since in
> the rare situations where a write lock is required for the linked list
> that j1939_socks_lock is protecting, the code does not attempt to
> acquire any more locks. This would break the circular lock dependency,
> where, for example, the current thread already locks j1939_socks_lock
> and attempts to acquire sk_session_queue_lock, and at the same time,
> another thread attempts to acquire j1939_socks_lock while holding
> sk_session_queue_lock.

Just FYI, Eric recently did a great job removing most rwlock in
networking code, see commit 0daf07e52709 ("raw: convert raw sockets to
RCU").

I'm unsure if the same reasoning apply to can, but perhaps you could
consider converting this code to RCU for net-next.

Cheers,

Paolo