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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 08 Sep 2022 16:44:58 +0200 From: Paolo Abeni <pabeni@...hat.com> To: Matthieu Baerts <matthieu.baerts@...sares.net>, menglong8.dong@...il.com Cc: mathew.j.martineau@...ux.intel.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, fw@...len.de, peter.krystad@...ux.intel.com, netdev@...r.kernel.org, mptcp@...ts.linux.dev, linux-kernel@...r.kernel.org, Menglong Dong <imagedong@...cent.com>, Jiang Biao <benbjiang@...cent.com>, Mengen Sun <mengensun@...cent.com> Subject: Re: [PATCH net v3] net: mptcp: fix unreleased socket in accept queue On Thu, 2022-09-08 at 15:56 +0200, Matthieu Baerts wrote: > Hi Menglong, > > On 07/09/2022 13:11, menglong8.dong@...il.com wrote: > > From: Menglong Dong <imagedong@...cent.com> > > > > The mptcp socket and its subflow sockets in accept queue can't be > > released after the process exit. > > > > While the release of a mptcp socket in listening state, the > > corresponding tcp socket will be released too. Meanwhile, the tcp > > socket in the unaccept queue will be released too. However, only init > > subflow is in the unaccept queue, and the joined subflow is not in the > > unaccept queue, which makes the joined subflow won't be released, and > > therefore the corresponding unaccepted mptcp socket will not be released > > to. > > Thank you for the v3. > > Unfortunately, our CI found a possible recursive locking: > > > - KVM Validation: debug: > > - Unstable: 1 failed test(s): selftest_mptcp_join - Critical: 1 Call Trace(s) ❌: > > - Task: https://cirrus-ci.com/task/5418283233968128 > > - Summary: https://api.cirrus-ci.com/v1/artifact/task/5418283233968128/summary/summary.txt > > https://lore.kernel.org/mptcp/4e6d3d9e-1f1a-23ae-cb56-2d4f043f17ae@gmail.com/T/#u > > Do you mind looking at it please? Ah, that is actually a false positive, but we must silence it. The main point is that the lock_sock() in mptcp_close() rightfully lacks the _nested annotation. Instead of adding such annotation only for this call site, which would be both ugly and dangerous, I suggest to factor_out from mptcp_close() all the code the run under the socket lock, say in: bool __mptcp_close(struct sock *sk, long timeout) // return true if the caller need to cancel the mptcp worker // (outside the socket lock) and then in mptcp_subflow_queue_clean(): sock_hold(sk); slow = lock_sock_fast_nested(sk); next = msk->dl_next; msk->first = NULL; msk->dl_next = NULL; do_cancel_work = __mptcp_close(sk, 0); unlock_sock_fast(sk, slow); if (do_cancel_work) mptcp_cancel_work(sk); sock_put(sk); All the above could require 2 different patches, 1 to factor-out the helper, and 1 to actually implement the fix. Cheers, Paolo
Powered by blists - more mailing lists