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
| ||
|
Message-ID: <CADxym3ZDow6UM6K3Nt_PDFq-4gfn8Hs0hCS62Dqsj4=tM_+hFQ@mail.gmail.com> Date: Fri, 9 Sep 2022 10:25:20 +0800 From: Menglong Dong <menglong8.dong@...il.com> To: Paolo Abeni <pabeni@...hat.com> Cc: Matthieu Baerts <matthieu.baerts@...sares.net>, 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, Sep 8, 2022 at 10:45 PM Paolo Abeni <pabeni@...hat.com> wrote: > > 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. > Thanks for your explanation! As Matthieu said, I'll send the next version to the MPTCP mailing list only. Thanks! Menglong Dong > Cheers, > > Paolo >
Powered by blists - more mailing lists