As discussed on [1], an additional check is done to catch local software
bug.
This patch is supposed to land only in our tree, for both 'export' and
'export-net' branches, because the warning could be triggered by bugged/
malicious peer. We want it in our tree for our CI to detect internal
bugs.
Link: https://lore.kernel.org/all/20240215-mptcp-fix-bogus-pr-warn-v1-1-d14c10312820@kernel.org/ [1]
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
net/mptcp/options.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 27ca42c77b02..5926955625cf 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -982,7 +982,8 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
if (unlikely(!READ_ONCE(msk->pm.server_side)))
- pr_warn_once("bogus mpc option on established client sk");
+ /* DO-NOT-MERGE: use WARN i/o pr_warn: only for MPTCP export */
+ WARN_ONCE(1, "bogus mpc option on established client sk");
set_fully_established:
mptcp_data_lock((struct sock *)msk);
---
base-commit: 65bda47e218b37c3dc919d3ccbd19dfad87a99e5
change-id: 20240216-mptcp-warn-bogus-pr-warn-6dcc88038246
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
Hi Mat, On 16/02/2024 13:04, Matthieu Baerts (NGI0) wrote: > As discussed on [1], an additional check is done to catch local software > bug. > > This patch is supposed to land only in our tree, for both 'export' and > 'export-net' branches, because the warning could be triggered by bugged/ > malicious peer. We want it in our tree for our CI to detect internal > bugs. Thank you for the review! Now in our tree, for both -net and -next: New patches for t/upstream: - 735e80ba1492: tg:msg: remove 'use kmalloc on kasan build' - 91351bfa97db: "squashed" in "DO-NOT-MERGE: mptcp: improve code coverage for CI" - 359220a914aa: tg:msg: add 'warn in case of bogus mpc option on established client sk' - Results: f2fb9bec2195..d5aedc5b25ce (export) New patches for t/upstream-net: - a1880660e156: tg:msg: remove 'use kmalloc on kasan build' - 1c78093d056a: Squash to "DO-NOT-MERGE: mptcp: improve code coverage for CI" - 5405ccb66c00: tg:msg: add 'warn in case of bogus mpc option on established client sk' - Results: 9dcf86462a59..22c802d10725 (export-net) Cheers, Matt -- Sponsored by the NGI0 Core fund.
On Fri, 16 Feb 2024, Matthieu Baerts (NGI0) wrote: > As discussed on [1], an additional check is done to catch local software > bug. > > This patch is supposed to land only in our tree, for both 'export' and > 'export-net' branches, because the warning could be triggered by bugged/ > malicious peer. We want it in our tree for our CI to detect internal > bugs. > > Link: https://lore.kernel.org/all/20240215-mptcp-fix-bogus-pr-warn-v1-1-d14c10312820@kernel.org/ [1] > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Looks good to squash. Would be good to keep some of the above info in the post-squash commit message for context (especially the lore link). One other thing, I noticed that this is mentioned in the "DO-NOT-MERGE: mptcp: improve code coverage for CI" commit message: """ mptcp: use kmalloc on kasan build Helps detection UaF, which apparently kasan misses with kmem_cache allocator. We also need to always set the SOCK_RCU_FREE flag, to preserved the current code leveraging SLAB_TYPESAFE_BY_RCU. This latter change will make unreachable some existing errors path, but I don't see other options. """ But it looks like the code for that disappeared between export/20230405T131056 and export/20230405T132954 (also missing on export-net). I didn't find mention of this in the mailing list. A topgit glitch? - Mat > --- > net/mptcp/options.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > index 27ca42c77b02..5926955625cf 100644 > --- a/net/mptcp/options.c > +++ b/net/mptcp/options.c > @@ -982,7 +982,8 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk, > WRITE_ONCE(msk->pm.remote_deny_join_id0, true); > > if (unlikely(!READ_ONCE(msk->pm.server_side))) > - pr_warn_once("bogus mpc option on established client sk"); > + /* DO-NOT-MERGE: use WARN i/o pr_warn: only for MPTCP export */ > + WARN_ONCE(1, "bogus mpc option on established client sk"); > > set_fully_established: > mptcp_data_lock((struct sock *)msk); > > --- > base-commit: 65bda47e218b37c3dc919d3ccbd19dfad87a99e5 > change-id: 20240216-mptcp-warn-bogus-pr-warn-6dcc88038246 > > Best regards, > -- > Matthieu Baerts (NGI0) <matttbe@kernel.org> > > >
Hi Mat, On 16/02/2024 21:21, Mat Martineau wrote: > On Fri, 16 Feb 2024, Matthieu Baerts (NGI0) wrote: > >> As discussed on [1], an additional check is done to catch local software >> bug. >> >> This patch is supposed to land only in our tree, for both 'export' and >> 'export-net' branches, because the warning could be triggered by bugged/ >> malicious peer. We want it in our tree for our CI to detect internal >> bugs. >> >> Link: >> https://lore.kernel.org/all/20240215-mptcp-fix-bogus-pr-warn-v1-1-d14c10312820@kernel.org/ [1] >> Suggested-by: Paolo Abeni <pabeni@redhat.com> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > Looks good to squash. Thank you for the review! > Would be good to keep some of the above info in > the post-squash commit message for context (especially the lore link). Yes, good point, I will update the '.topmsg' file. > One other thing, I noticed that this is mentioned in the "DO-NOT-MERGE: > mptcp: improve code coverage for CI" commit message: > > """ > mptcp: use kmalloc on kasan build > > Helps detection UaF, which apparently kasan misses with kmem_cache > allocator. > > We also need to always set the SOCK_RCU_FREE flag, to preserved the > current code leveraging SLAB_TYPESAFE_BY_RCU. This latter change will > make unreachable some existing errors path, but I don't see other > options. > > """ > > But it looks like the code for that disappeared between > export/20230405T131056 and export/20230405T132954 (also missing on > export-net). I didn't find mention of this in the mailing list. A topgit > glitch? Oops, I forgot to update the commit message back then. Here is the modification I did in TopGit: $ git log --first-parent --no-merges $(./.tg-get-topic.sh "DO-NOT-MERGE: mptcp: improve code coverage for CI") -2 (...) # (the first commit was an update of my email address) commit 0fd1a78a8c4d99054a1b47626d45df7f5d36e2ec Author: Matthieu Baerts <matttbe@kernel.org> Date: Wed Apr 5 15:23:56 2023 +0200 Revert "DO-NOT-MERGE: mptcp: use kmalloc on kasan build" This reverts commit 4edee9adfa20120c18e41c26e2ca0a97ead0d93f. This piece of code was causing issues, see: https://github.com/multipath-tcp/mptcp_net-next/issues/371#issuecomment-1496226126 It sounds safer to revert it then. Suggested-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> I can fix that! Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.