[PATCH mptcp-next v3 0/3] mptcp: check the protocol with DEBUG_NET

[PATCH mptcp-next v3 0/3] mptcp: check the protocol with DEBUG_NET

[Matthieu Baerts (NGI0)]

Recently, Paolo fixed a bug where a TCP-specific helper was used with an
MPTCP socket [1]. The bug was not detected by fuzzer or static analysis.

Following this, it has been suggested to add a check, only in debug
mode. This is what this series is doing.

The series has been split to be upstreamed: a preparation patch for
MPTCP, the modification for TCP, then for MPTCP. It is not clear if it
would be OK to add that upstream. If not, we can squash these three
patches in "DO-NOT-MERGE: mptcp: improve code coverage for CI" commit we
have in our export tree.

Note that the MPTCP Token kUnit test needs to be adapted for this new
check. This is what is done in patch 1/3.

Link: https://lore.kernel.org/mptcp/35875ef9cb7194563b580e14c71cc8cb065f846c.1706043786.git.pabeni@redhat.com/ [1]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v3:
- Patch 2 and 3 have been modified, please the changelog on each patch
- Link to v2: https://lore.kernel.org/r/20240201-mptcp-check-protocol-v2-0-1e253ef51990@kernel.org

Changes in v2:
- Patch 2 and 3 have been modified, please the changelog on each patch
- Link to v1: https://lore.kernel.org/r/20240131-mptcp-check-protocol-v1-0-a06067f0bd08@kernel.org

---
Matthieu Baerts (NGI0) (3):
      mptcp: token kunit: set protocol
      mptcp: check the protocol in tcp_sk() with DEBUG_NET
      mptcp: check the protocol in mptcp_sk() with DEBUG_NET

 net/mptcp/protocol.h   | 14 ++++++++++++++
 net/mptcp/token_test.c |  7 ++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)
---
base-commit: c2469b38e369c3f2b9577beeb9470cc757abc1b9
change-id: 20240131-mptcp-check-protocol-e32e53d04a75

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next v3 1/3] mptcp: token kunit: set protocol

[Matthieu Baerts (NGI0)]

As it would be done when initiating an MPTCP sock.

This is not strictly needed for this test, but it will be when a later
patch will check if the right protocol is being used when calling
mptcp_sk().

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/token_test.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/token_test.c b/net/mptcp/token_test.c
index bfff53e668da..4fc39fa2e262 100644
--- a/net/mptcp/token_test.c
+++ b/net/mptcp/token_test.c
@@ -52,14 +52,19 @@ static struct mptcp_subflow_context *build_ctx(struct kunit *test)
 static struct mptcp_sock *build_msk(struct kunit *test)
 {
 	struct mptcp_sock *msk;
+	struct sock *sk;
 
 	msk = kunit_kzalloc(test, sizeof(struct mptcp_sock), GFP_USER);
 	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, msk);
 	refcount_set(&((struct sock *)msk)->sk_refcnt, 1);
 	sock_net_set((struct sock *)msk, &init_net);
 
+	sk = (struct sock *)msk;
+
 	/* be sure the token helpers can dereference sk->sk_prot */
-	((struct sock *)msk)->sk_prot = &tcp_prot;
+	sk->sk_prot = &tcp_prot;
+	sk->sk_protocol = IPPROTO_MPTCP;
+
 	return msk;
 }
 

-- 
2.43.0

[PATCH mptcp-next v3 2/3] mptcp: check the protocol in tcp_sk() with DEBUG_NET

[Matthieu Baerts (NGI0)]

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

Re: [PATCH mptcp-next v3 2/3] mptcp: check the protocol in tcp_sk() with DEBUG_NET

[Mat Martineau]

On Wed, 7 Feb 2024, Matthieu Baerts (NGI0) wrote:

> 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);	\
> +})

Hi Matthieu -

Could make checkpatch happy with:

#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);	\
})

(same idea for patch 3)

- Mat

> +#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
>
>
>

Re: [PATCH mptcp-next v3 2/3] mptcp: check the protocol in tcp_sk() with DEBUG_NET

[Matthieu Baerts]

Hi Mat,

On 14/02/2024 20:03, Mat Martineau wrote:
> On Wed, 7 Feb 2024, Matthieu Baerts (NGI0) wrote:
> 
>> 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);    \
>> +})
> 
> Hi Matthieu -
> 
> Could make checkpatch happy with:
> 
> #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);    \
> })
> 
> (same idea for patch 3)

Good idea! Better be safe than sorry :)

I just sent a v4. While at it, I also removed the extra parenthesis
around '_ptr' in 'WARN_ON((_ptr)...)': they are no longer needed now.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH mptcp-next v3 3/3] mptcp: check the protocol in mptcp_sk() with DEBUG_NET

[Matthieu Baerts (NGI0)]

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

This is similar to the parent commit, where it is easy to use mptcp_sk()
with a TCP sock, e.g. with a subflow sk.

So a new simple check is done when CONFIG_DEBUG_NET is enabled to tell
kernel devs when a non-MPTCP socket is being used as an MPTCP one.
'mptcp_sk()' macro is then defined differently: with an extra WARN to
complain when an unexpected socket is being used.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
  v2:
    - Use a macro instead of an inlined function (Paolo)
  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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 6b62a7f35dd9..2b837676d1fc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -355,9 +355,14 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 	WARN_ON((ptr)->sk_protocol != IPPROTO_TCP);				\
 	container_of_const(ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
 })
-#endif
+#define mptcp_sk(ptr) ({						\
+	WARN_ON((ptr)->sk_protocol != IPPROTO_MPTCP);			\
+	container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk);	\
+})
 
+#else /* !CONFIG_DEBUG_NET */
 #define mptcp_sk(ptr) container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk)
+#endif
 
 /* the msk socket don't use the backlog, also account for the bulk
  * free memory

-- 
2.43.0

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7814594663

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/7511efd6fe57


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5671751590346752
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5671751590346752/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5108801636925440
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5108801636925440/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/7511efd6fe57


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

[PATCH mptcp-next v4 3/3] mptcp: check the protocol in mptcp_sk() with DEBUG_NET

[Matthieu Baerts (NGI0)]

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

This is similar to the parent commit, where it is easy to use mptcp_sk()
with a TCP sock, e.g. with a subflow sk.

So a new simple check is done when CONFIG_DEBUG_NET is enabled to tell
kernel devs when a non-MPTCP socket is being used as an MPTCP one.
'mptcp_sk()' macro is then defined differently: with an extra WARN to
complain when an unexpected socket is being used.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
  - v2:
    - Use a macro instead of an inlined function (Paolo)
  - 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?
  - v4:
    - avoid reusing 'ptr' to fix checkpatch warning (Mat)
    - remove extra parenthesis in WARN_ON, no longer needed
---
 net/mptcp/protocol.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 32cf98bd2961..459859b107ba 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -356,9 +356,15 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 	WARN_ON(_ptr->sk_protocol != IPPROTO_TCP);				\
 	container_of_const(_ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
 })
-#endif
+#define mptcp_sk(ptr) ({						\
+	typeof(ptr) _ptr = (ptr);					\
+	WARN_ON(_ptr->sk_protocol != IPPROTO_MPTCP);			\
+	container_of_const(_ptr, struct mptcp_sock, sk.icsk_inet.sk);	\
+})
 
+#else /* !CONFIG_DEBUG_NET */
 #define mptcp_sk(ptr) container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk)
+#endif
 
 /* the msk socket don't use the backlog, also account for the bulk
  * free memory

-- 
2.43.0

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7915472729

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/96cb1155ed44


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Unstable: 1 failed test(s): packetdrill_regressions 🔴:
  - Task: https://cirrus-ci.com/task/5212375712792576
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5212375712792576/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6338275619635200
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6338275619635200/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/96cb1155ed44


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7923641372

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e79de30e413b


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5269484584304640
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5269484584304640/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6395384491147264
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6395384491147264/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e79de30e413b


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

[PATCH mptcp-next v2 3/3] mptcp: check the protocol in mptcp_sk() with DEBUG_NET

[Matthieu Baerts (NGI0)]

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

This is similar to the parent commit, where it is easy to use mptcp_sk()
with a TCP sock, e.g. with a subflow sk.

So a new simple check is done when CONFIG_DEBUG_NET is enabled to tell
kernel devs when a non-MPTCP socket is being used as an MPTCP one.
'mptcp_sk()' macro is then defined differently: with an extra WARN to
complain when an unexpected socket is being used.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
  v2:
    - Use a macro instead of an inlined function (Paolo)
---
 net/mptcp/protocol.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f2473d9acae6..defccef59b3e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -355,9 +355,14 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 	WARN_ON(ptr->sk_protocol != IPPROTO_TCP);				\
 	container_of_const(ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
 })
-#endif
+#define mptcp_sk(ptr) ({						\
+	WARN_ON(ptr->sk_protocol != IPPROTO_MPTCP);			\
+	container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk);	\
+})
 
+#else /* !CONFIG_DEBUG_NET */
 #define mptcp_sk(ptr) container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk)
+#endif
 
 /* the msk socket don't use the backlog, also account for the bulk
  * free memory

-- 
2.43.0

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7744410100

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/05d35e87fc2b


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: mptcp: check the protocol in mptcp_sk() with DEBUG_NET: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5743237663555584
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5743237663555584/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5180287710134272
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5180287710134272/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/05d35e87fc2b


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)