[PATCH bpf-next 1/2] bpf: net: Avoid loading module when calling bpf_setsockopt(TCP_CONGESTION)

[PATCH bpf-next 1/2] bpf: net: Avoid loading module when calling bpf_setsockopt(TCP_CONGESTION)

[Martin KaFai Lau]

When bpf prog changes tcp-cc by calling bpf_setsockopt(TCP_CONGESTION),
it should not try to load module which may be a blocking
operation.  This details was correct in the v1 [0] but missed by
mistake in the later revision in
commit cb388e7ee3a8 ("bpf: net: Change do_tcp_setsockopt() to use the sockopt's lock_sock() and capable()")

This patch fixes it by checking the has_current_bpf_ctx().

[0]: https://lore.kernel.org/bpf/20220727060921.2373314-1-kafai@fb.com/

Fixes: cb388e7ee3a8 ("bpf: net: Change do_tcp_setsockopt() to use the sockopt's lock_sock() and capable()")
Signed-off-by: Martin KaFai Lau <martin.lau@linux.dev>
---
 net/ipv4/tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a6986f201f92..f0d79ea45ac8 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3503,7 +3503,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		name[val] = 0;
 
 		sockopt_lock_sock(sk);
-		err = tcp_set_congestion_control(sk, name, true,
+		err = tcp_set_congestion_control(sk, name, !has_current_bpf_ctx(),
 						 sockopt_ns_capable(sock_net(sk)->user_ns,
 								    CAP_NET_ADMIN));
 		sockopt_release_sock(sk);
-- 
2.30.2

[PATCH bpf-next 2/2] selftest/bpf: Ensure no module loading in bpf_setsockopt(TCP_CONGESTION)

[Martin KaFai Lau]

This patch adds a test to ensure
bpf_setsockopt(TCP_CONGESTION, "not_exist")
will not trigger the kernel module autoload.

Before the fix:
[   40.535829] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:274
...
[   40.552134]  tcp_ca_find_autoload.constprop.0+0xcb/0x200
[   40.552689]  tcp_set_congestion_control+0x99/0x7b0
[   40.553203]  do_tcp_setsockopt+0x3ed/0x2240
...
[   40.556041]  __bpf_setsockopt+0x124/0x640

Signed-off-by: Martin KaFai Lau <martin.lau@linux.dev>
---
 tools/testing/selftests/bpf/progs/setget_sockopt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
index 40606ef47a38..79debf3c2f44 100644
--- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
+++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
@@ -32,6 +32,7 @@ struct sockopt_test {
 	unsigned int flip:1;
 };
 
+static const char not_exist_cc[] = "not_exist";
 static const char cubic_cc[] = "cubic";
 static const char reno_cc[] = "reno";
 
@@ -307,6 +308,9 @@ static int bpf_test_tcp_sockopt(__u32 i, struct loop_ctx *lc)
 		const char *new_cc;
 		int new_cc_len;
 
+		if (!bpf_setsockopt(ctx, IPPROTO_TCP, TCP_CONGESTION,
+				    (void *)not_exist_cc, sizeof(not_exist_cc)))
+			return 1;
 		if (bpf_getsockopt(ctx, IPPROTO_TCP, TCP_CONGESTION, old_cc, sizeof(old_cc)))
 			return 1;
 		if (!bpf_strncmp(old_cc, sizeof(old_cc), cubic_cc)) {
-- 
2.30.2

Re: [PATCH bpf-next 1/2] bpf: net: Avoid loading module when calling bpf_setsockopt(TCP_CONGESTION)

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Tue, 30 Aug 2022 16:19:46 -0700 you wrote:
> When bpf prog changes tcp-cc by calling bpf_setsockopt(TCP_CONGESTION),
> it should not try to load module which may be a blocking
> operation.  This details was correct in the v1 [0] but missed by
> mistake in the later revision in
> commit cb388e7ee3a8 ("bpf: net: Change do_tcp_setsockopt() to use the sockopt's lock_sock() and capable()")
> 
> This patch fixes it by checking the has_current_bpf_ctx().
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: net: Avoid loading module when calling bpf_setsockopt(TCP_CONGESTION)
    https://git.kernel.org/bpf/bpf-next/c/84e5a0f208ca
  - [bpf-next,2/2] selftest/bpf: Ensure no module loading in bpf_setsockopt(TCP_CONGESTION)
    https://git.kernel.org/bpf/bpf-next/c/197072945a70

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html