mptcp: fix setsockopt(IP_TOS) subflow locking

[PATCH mptcp-net] mptcp: fix setsockopt(IP_TOS) subflow locking

Paolo Abeni posted 1 patch 5 months, 4 weeks ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/f2cf8a00fdf178f314b92c8809ca9deed98464fa.1699466615.git.pabeni@redhat.com

Maintainers: Matthieu Baerts <matttbe@kernel.org>, Mat Martineau <martineau@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Poorva Sonparote <psonparo@redhat.com>

net/mptcp/sockopt.c | 3 +++
1 file changed, 3 insertions(+)

Expand all Fold all

[PATCH mptcp-net] mptcp: fix setsockopt(IP_TOS) subflow locking

Posted by Paolo Abeni 5 months, 4 weeks ago

The mptcp implementation of the IP_TOS socket option uses the lockless
variant of the TOS manipulation helper and does not hold suck lock at
the helper invocation time.

Add the required locking.

Fixes: ffcacff87cd6 ("mptcp: Support for IP_TOS for MPTCP setsockopt()")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/457
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/sockopt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8d485c40585a..cabe856b2a45 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -738,8 +738,11 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
 	val = READ_ONCE(inet_sk(sk)->tos);
 	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+		bool slow;
 
+		slow = lock_sock_fast(ssk);
 		__ip_sock_set_tos(ssk, val);
+		unlock_sock_fast(ssk, slow);
 	}
 	release_sock(sk);
 
-- 
2.41.0

Re: [PATCH mptcp-net] mptcp: fix setsockopt(IP_TOS) subflow locking

Posted by Matthieu Baerts 5 months, 3 weeks ago

Hi Paolo, Mat,

On 08/11/2023 19:26, Paolo Abeni wrote:
> The mptcp implementation of the IP_TOS socket option uses the lockless
> variant of the TOS manipulation helper and does not hold suck lock at
> the helper invocation time.
> 
> Add the required locking.

Thank you for the patch and the review!

Now in our tree (fixes for -net), with 's/suck/such/', because typos
suck...:

New patches for t/upstream-net and t/upstream:
- c69c00a783ed: mptcp: fix setsockopt(IP_TOS) subflow locking
- Results: 688025cbce11..52a2364fffc9 (export-net)
- Results: 4d89b70d7306..351099187dd4 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20231109T161702
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20231109T161702

Cheers,
Matt

Re: [PATCH mptcp-net] mptcp: fix setsockopt(IP_TOS) subflow locking

Posted by Mat Martineau 5 months, 4 weeks ago

On Wed, 8 Nov 2023, Paolo Abeni wrote:

> The mptcp implementation of the IP_TOS socket option uses the lockless
> variant of the TOS manipulation helper and does not hold suck lock at

(matttbe: please fix "such" typo when applying)

> the helper invocation time.
>
> Add the required locking.
>
> Fixes: ffcacff87cd6 ("mptcp: Support for IP_TOS for MPTCP setsockopt()")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/457
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/sockopt.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 8d485c40585a..cabe856b2a45 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -738,8 +738,11 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
> 	val = READ_ONCE(inet_sk(sk)->tos);
> 	mptcp_for_each_subflow(msk, subflow) {
> 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		bool slow;
>
> +		slow = lock_sock_fast(ssk);
> 		__ip_sock_set_tos(ssk, val);
> +		unlock_sock_fast(ssk, slow);
> 	}
> 	release_sock(sk);
>

Looks good to me with commit message typo fix:

Reviewed-by: Mat Martineau <martineau@kernel.org>