[PATCH mptcp-next 0/4] mptcp: Spring cleaning

[PATCH mptcp-next 0/4] mptcp: Spring cleaning

[Matthieu Baerts (NGI0)]

This series fixes some warnings reported by CheckPatch and CLang.

The behaviour is not supposed to be modified, just some simple fixes.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (4):
      mptcp: prefer strscpy over strcpy
      mptcp: move mptcp_pm_gen.h's include
      mptcp: remove unnecessary else statements
      mptcp: include inet_common in mib.h

 net/mptcp/ctrl.c         |  2 +-
 net/mptcp/mib.h          |  2 ++
 net/mptcp/pm_netlink.c   |  1 +
 net/mptcp/pm_userspace.c |  1 +
 net/mptcp/protocol.c     |  5 +++--
 net/mptcp/protocol.h     |  2 --
 net/mptcp/sockopt.c      |  2 +-
 net/mptcp/subflow.c      | 32 +++++++++++++++++---------------
 8 files changed, 26 insertions(+), 21 deletions(-)
---
base-commit: c3c3514169a8e548fba51d0694b86a72f89acce0
change-id: 20240502-mptcp-cleanup-clang-checkpatch-8eb4a4843ff6

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

[PATCH mptcp-next 1/4] mptcp: prefer strscpy over strcpy

[Matthieu Baerts (NGI0)]

strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of misbehaviors. The safe replacement is strscpy() [1].

This is in preparation of a possible future step where all strcpy() uses
will be removed in favour of strscpy() [2].

This fixes CheckPatch warnings:

  WARNING: Prefer strscpy over strcpy

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
Link: https://github.com/KSPP/linux/issues/88 [2]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/ctrl.c     | 2 +-
 net/mptcp/protocol.c | 5 +++--
 net/mptcp/sockopt.c  | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 8d661156ab8c..3d9d26b29bcd 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -92,7 +92,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->allow_join_initial_addr_port = 1;
 	pernet->stale_loss_cnt = 4;
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
-	strcpy(pernet->scheduler, "default");
+	strscpy(pernet->scheduler, "default", sizeof(pernet->scheduler));
 }
 
 #ifdef CONFIG_SYSCTL
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 62403f93c4d7..579031c60937 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2814,7 +2814,8 @@ static void mptcp_ca_reset(struct sock *sk)
 	struct inet_connection_sock *icsk = inet_csk(sk);
 
 	tcp_assign_congestion_control(sk);
-	strcpy(mptcp_sk(sk)->ca_name, icsk->icsk_ca_ops->name);
+	strscpy(mptcp_sk(sk)->ca_name, icsk->icsk_ca_ops->name,
+		sizeof(mptcp_sk(sk)->ca_name));
 
 	/* no need to keep a reference to the ops, the name will suffice */
 	tcp_cleanup_congestion_control(sk);
@@ -4169,7 +4170,7 @@ int __init mptcp_proto_v6_init(void)
 	int err;
 
 	mptcp_v6_prot = mptcp_prot;
-	strcpy(mptcp_v6_prot.name, "MPTCPv6");
+	strscpy(mptcp_v6_prot.name, "MPTCPv6", sizeof(mptcp_v6_prot.name));
 	mptcp_v6_prot.slab = NULL;
 	mptcp_v6_prot.obj_size = sizeof(struct mptcp6_sock);
 	mptcp_v6_prot.ipv6_pinfo_offset = offsetof(struct mptcp6_sock, np);
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index b0d1dc4df0c1..2edaf1a16005 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -618,7 +618,7 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
 	}
 
 	if (ret == 0)
-		strcpy(msk->ca_name, name);
+		strscpy(msk->ca_name, name, sizeof(msk->ca_name));
 
 	release_sock(sk);
 	return ret;

-- 
2.43.0

[PATCH mptcp-next 2/4] mptcp: move mptcp_pm_gen.h's include

[Matthieu Baerts (NGI0)]

Nothing from protocol.h depends on mptcp_pm_gen.h, only code from
pm_netlink.c and pm_userspace.c depends on it.

So this include can be moved where it is needed to avoid a "unused
includes" warning.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   | 1 +
 net/mptcp/pm_userspace.c | 1 +
 net/mptcp/protocol.h     | 2 --
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 5c17d39146ea..7f53e022e27e 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -14,6 +14,7 @@
 
 #include "protocol.h"
 #include "mib.h"
+#include "mptcp_pm_gen.h"
 
 static int pm_nl_pernet_id;
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 9f5d422d5ef6..f0a4590506c6 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -6,6 +6,7 @@
 
 #include "protocol.h"
 #include "mib.h"
+#include "mptcp_pm_gen.h"
 
 void mptcp_free_local_addr_list(struct mptcp_sock *msk)
 {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ffd00fb45433..2d40629e8231 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -14,8 +14,6 @@
 #include <net/genetlink.h>
 #include <net/rstreason.h>
 
-#include "mptcp_pm_gen.h"
-
 #define MPTCP_SUPPORTED_VERSION	1
 
 /* MPTCP option bits */

-- 
2.43.0

[PATCH mptcp-next 3/4] mptcp: remove unnecessary else statements

[Matthieu Baerts (NGI0)]

The 'else' statements are not needed here, because their previous 'if'
block ends with a 'return'.

This fixes CheckPatch warnings:

  WARNING: else is not generally useful after a break or return

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/subflow.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 97ec44d1df30..7987342f4526 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1117,6 +1117,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 	}
 
 	if (mpext->data_fin == 1) {
+		u64 data_fin_seq;
+
 		if (data_len == 1) {
 			bool updated = mptcp_update_rcv_data_fin(msk, mpext->data_seq,
 								 mpext->dsn64);
@@ -1129,26 +1131,26 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 				 */
 				skb_ext_del(skb, SKB_EXT_MPTCP);
 				return MAPPING_OK;
-			} else {
-				if (updated)
-					mptcp_schedule_work((struct sock *)msk);
-
-				return MAPPING_DATA_FIN;
 			}
-		} else {
-			u64 data_fin_seq = mpext->data_seq + data_len - 1;
 
-			/* If mpext->data_seq is a 32-bit value, data_fin_seq
-			 * must also be limited to 32 bits.
-			 */
-			if (!mpext->dsn64)
-				data_fin_seq &= GENMASK_ULL(31, 0);
+			if (updated)
+				mptcp_schedule_work((struct sock *)msk);
 
-			mptcp_update_rcv_data_fin(msk, data_fin_seq, mpext->dsn64);
-			pr_debug("DATA_FIN with mapping seq=%llu dsn64=%d",
-				 data_fin_seq, mpext->dsn64);
+			return MAPPING_DATA_FIN;
 		}
 
+		data_fin_seq = mpext->data_seq + data_len - 1;
+
+		/* If mpext->data_seq is a 32-bit value, data_fin_seq must also
+		 * be limited to 32 bits.
+		 */
+		if (!mpext->dsn64)
+			data_fin_seq &= GENMASK_ULL(31, 0);
+
+		mptcp_update_rcv_data_fin(msk, data_fin_seq, mpext->dsn64);
+		pr_debug("DATA_FIN with mapping seq=%llu dsn64=%d",
+			 data_fin_seq, mpext->dsn64);
+
 		/* Adjust for DATA_FIN using 1 byte of sequence space */
 		data_len--;
 	}

-- 
2.43.0

[PATCH mptcp-next 4/4] mptcp: include inet_common in mib.h

[Matthieu Baerts (NGI0)]

So this file is now self-contained: it can be compiled alone with
analytic tools.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/mib.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index dd7fd1f246b5..2704afd0dfe4 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
+#include <net/inet_common.h>
+
 enum linux_mptcp_mib_field {
 	MPTCP_MIB_NUM = 0,
 	MPTCP_MIB_MPCAPABLEPASSIVE,	/* Received SYN with MP_CAPABLE */

-- 
2.43.0

Re: [PATCH mptcp-next 0/4] mptcp: Spring cleaning

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/8928081434

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/85cda034c68b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=849979


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: [PATCH mptcp-next 0/4] mptcp: Spring cleaning

[Geliang Tang]

Hi Matt,

On Thu, May 02, 2024 at 07:23:03PM +0200, Matthieu Baerts (NGI0) wrote:
> This series fixes some warnings reported by CheckPatch and CLang.
> 
> The behaviour is not supposed to be modified, just some simple fixes.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This series looks good to me, thanks.

Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> ---
> Matthieu Baerts (NGI0) (4):
>       mptcp: prefer strscpy over strcpy
>       mptcp: move mptcp_pm_gen.h's include
>       mptcp: remove unnecessary else statements
>       mptcp: include inet_common in mib.h
> 
>  net/mptcp/ctrl.c         |  2 +-
>  net/mptcp/mib.h          |  2 ++
>  net/mptcp/pm_netlink.c   |  1 +
>  net/mptcp/pm_userspace.c |  1 +
>  net/mptcp/protocol.c     |  5 +++--
>  net/mptcp/protocol.h     |  2 --
>  net/mptcp/sockopt.c      |  2 +-
>  net/mptcp/subflow.c      | 32 +++++++++++++++++---------------
>  8 files changed, 26 insertions(+), 21 deletions(-)
> ---
> base-commit: c3c3514169a8e548fba51d0694b86a72f89acce0
> change-id: 20240502-mptcp-cleanup-clang-checkpatch-8eb4a4843ff6
> 
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 

Re: [PATCH mptcp-next 0/4] mptcp: Spring cleaning

[Matthieu Baerts]

Hi Geliang,

On 05/05/2024 02:09, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, May 02, 2024 at 07:23:03PM +0200, Matthieu Baerts (NGI0) wrote:
>> This series fixes some warnings reported by CheckPatch and CLang.
>>
>> The behaviour is not supposed to be modified, just some simple fixes.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> This series looks good to me, thanks.
> 
> Reviewed-by: Geliang Tang <geliang@kernel.org>

Thank you for the reviews!

Now in our tree:

New patches for t/upstream:
- 30bcdc95bba9: mptcp: prefer strscpy over strcpy
- a34ad3c16bea: mptcp: move mptcp_pm_gen.h's include
- ee96a2272093: mptcp: remove unnecessary else statements
- 1122ef798833: mptcp: include inet_common in mib.h
- Results: 553805a44319..77024827f43c (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/0c710353718af7825e121b25983bc352ccc64024/checks

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