All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
To: mptcp@lists.linux.dev
Cc: Paolo Abeni <pabeni@redhat.com>,
	 "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
Subject: [PATCH mptcp-next v3 2/3] mptcp: check the protocol in tcp_sk() with DEBUG_NET
Date: Wed, 07 Feb 2024 12:56:24 +0100	[thread overview]
Message-ID: <20240207-mptcp-check-protocol-v3-2-77c69c596f55@kernel.org> (raw)
In-Reply-To: <20240207-mptcp-check-protocol-v3-0-77c69c596f55@kernel.org>

Fuzzers and static checkers might not detect when tcp_sk() is used with
a non tcp_sock structure.

This kind of mistake already happened a few times with MPTCP: when
wrongly using TCP-specific helpers with mptcp_sock pointers. On the
other hand, there are many 'tcp_xxx()' helpers that are taking a 'struct
sock' pointer as arguments, and some of them are only looking at fields
from 'struct sock', and nothing from 'struct tcp_sock'. It is then
tempting to use them with a 'struct mptcp_sock'.

So a new simple check is done when CONFIG_DEBUG_NET is enabled to tell
kernel devs when a non-TCP socket is being used as a TCP one. 'tcp_sk()'
macro is then re-defined to add a WARN when an unexpected socket is
being used.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
  - v2:
    - Move from include/linux/tcp.h to net/mptcp/protocol.h: specific
      to TCP (Paolo)
    - Use a macro instead of an inlined function (Paolo)
    - Adapt the commit message after the recent changes.
  - v3:
    - add parenthesis around 'ptr' (checkpatch)
    - there is still this check from checkpatch but I guess that's fine:
      Macro argument reuse 'ptr' - possible side-effects?
---
 net/mptcp/protocol.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index eefd1397106d..6b62a7f35dd9 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -348,6 +348,15 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 	sock_owned_by_me((const struct sock *)msk);
 }
 
+#ifdef CONFIG_DEBUG_NET
+/* MPTCP-specific: we might (indirectly) call this helper with the wrong sk */
+#undef tcp_sk
+#define tcp_sk(ptr) ({								\
+	WARN_ON((ptr)->sk_protocol != IPPROTO_TCP);				\
+	container_of_const(ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
+})
+#endif
+
 #define mptcp_sk(ptr) container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk)
 
 /* the msk socket don't use the backlog, also account for the bulk

-- 
2.43.0


  parent reply	other threads:[~2024-02-07 11:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 11:56 [PATCH mptcp-next v3 0/3] mptcp: check the protocol with DEBUG_NET Matthieu Baerts (NGI0)
2024-02-07 11:56 ` [PATCH mptcp-next v3 1/3] mptcp: token kunit: set protocol Matthieu Baerts (NGI0)
2024-02-07 11:56 ` Matthieu Baerts (NGI0) [this message]
2024-02-14 19:03   ` [PATCH mptcp-next v3 2/3] mptcp: check the protocol in tcp_sk() with DEBUG_NET Mat Martineau
2024-02-15 11:42     ` Matthieu Baerts
2024-02-07 11:56 ` [PATCH mptcp-next v3 3/3] mptcp: check the protocol in mptcp_sk() " Matthieu Baerts (NGI0)
2024-02-07 12:52   ` mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results MPTCP CI
2024-02-07 13:10   ` MPTCP CI

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240207-mptcp-check-protocol-v3-2-77c69c596f55@kernel.org \
    --to=matttbe@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.