All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
To: mptcp@lists.linux.dev, Mat Martineau <martineau@kernel.org>,
	 Geliang Tang <geliang@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
Cc: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
	 linux-kernel@vger.kernel.org,
	"Matthieu Baerts (NGI0)" <matttbe@kernel.org>
Subject: [PATCH net-next 3/8] mptcp: check the protocol in tcp_sk() with DEBUG_NET
Date: Fri, 23 Feb 2024 21:17:55 +0100	[thread overview]
Message-ID: <20240223-upstream-net-next-20240223-misc-improvements-v1-3-b6c8a10396bd@kernel.org> (raw)
In-Reply-To: <20240223-upstream-net-next-20240223-misc-improvements-v1-0-b6c8a10396bd@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.

Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f89b197a9f92..026ed360bd72 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -348,6 +348,16 @@ 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) ({								\
+	typeof(ptr) _ptr = (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-23 20:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23 20:17 [PATCH net-next 0/8] mptcp: various small improvements Matthieu Baerts (NGI0)
2024-02-23 20:17 ` [PATCH net-next 1/8] selftests: mptcp: lib: catch duplicated subtest entries Matthieu Baerts (NGI0)
2024-02-23 20:17 ` [PATCH net-next 2/8] mptcp: token kunit: set protocol Matthieu Baerts (NGI0)
2024-02-23 20:17 ` Matthieu Baerts (NGI0) [this message]
2024-02-23 20:17 ` [PATCH net-next 4/8] mptcp: check the protocol in mptcp_sk() with DEBUG_NET Matthieu Baerts (NGI0)
2024-02-23 20:17 ` [PATCH net-next 5/8] selftests: mptcp: netlink: drop duplicate var ret Matthieu Baerts (NGI0)
2024-02-23 20:17 ` [PATCH net-next 6/8] selftests: mptcp: simult flows: define missing vars Matthieu Baerts (NGI0)
2024-02-23 20:17 ` [PATCH net-next 7/8] selftests: mptcp: join: change capture/checksum as bool Matthieu Baerts (NGI0)
2024-02-23 20:18 ` [PATCH net-next 8/8] selftests: mptcp: diag: change timeout_poll to 30 Matthieu Baerts (NGI0)
2024-02-27  3:20 ` [PATCH net-next 0/8] mptcp: various small improvements patchwork-bot+netdevbpf

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=20240223-upstream-net-next-20240223-misc-improvements-v1-3-b6c8a10396bd@kernel.org \
    --to=matttbe@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geliang@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /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.