[PATCH net 1/2] bpf, sockmap: Do not ignore orig_len parameter

[PATCH net 1/2] bpf, sockmap: Do not ignore orig_len parameter

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Currently, sk_psock_verdict_recv() returns skb->len

This is problematic because tcp_read_sock() might have
passed orig_len < skb->len, due to the presence of TCP urgent data.

This causes an infinite loop from tcp_read_sock()

Followup patch will make tcp_read_sock() more robust vs bad actors.

Fixes: ef5659280eb1 ("bpf, sockmap: Allow skipping sk_skb parser program")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Jakub Sitnicki <jakub@cloudflare.com>
Tested-by: Jakub Sitnicki <jakub@cloudflare.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/skmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 8eb671c827f90f1f3d2514163fc82998c9906cb6..929a2b096b04e01b85bff0a69209413abe86102d 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -1153,7 +1153,7 @@ static int sk_psock_verdict_recv(read_descriptor_t *desc, struct sk_buff *skb,
 	struct sk_psock *psock;
 	struct bpf_prog *prog;
 	int ret = __SK_DROP;
-	int len = skb->len;
+	int len = orig_len;
 
 	/* clone here so sk_eat_skb() in tcp_read_sock does not drop our data */
 	skb = skb_clone(skb, GFP_ATOMIC);
-- 
2.35.1.574.g5d30c73bfb-goog

[PATCH net 2/2] tcp: make tcp_read_sock() more robust

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

If recv_actor() returns an incorrect value, tcp_read_sock()
might loop forever.

Instead, issue a one time warning and make sure to make progress.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Jakub Sitnicki <jakub@cloudflare.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/ipv4/tcp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 02cb275e5487d98b3e124ee102163aac47b2ad6d..28ff2a820f7c935234e5ab7ecd4ed95fb7c5712a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1684,11 +1684,13 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 				if (!copied)
 					copied = used;
 				break;
-			} else if (used <= len) {
-				seq += used;
-				copied += used;
-				offset += used;
 			}
+			if (WARN_ON_ONCE(used > len))
+				used = len;
+			seq += used;
+			copied += used;
+			offset += used;
+
 			/* If recv_actor drops the lock (e.g. TCP splice
 			 * receive) the skb pointer might be invalid when
 			 * getting here: tcp_collapse might have deleted it
-- 
2.35.1.574.g5d30c73bfb-goog

Re: [PATCH net 1/2] bpf, sockmap: Do not ignore orig_len parameter

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  2 Mar 2022 08:17:22 -0800 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Currently, sk_psock_verdict_recv() returns skb->len
> 
> This is problematic because tcp_read_sock() might have
> passed orig_len < skb->len, due to the presence of TCP urgent data.
> 
> [...]

Here is the summary with links:
  - [net,1/2] bpf, sockmap: Do not ignore orig_len parameter
    https://git.kernel.org/netdev/net/c/60ce37b03917
  - [net,2/2] tcp: make tcp_read_sock() more robust
    https://git.kernel.org/netdev/net/c/e3d5ea2c011e

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