| From 485b2e483d41e1673bb507e0a6549b821c9f7a22 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 12 Jul 2021 12:55:45 -0700 |
| Subject: bpf, sockmap: Fix potential memory leak on unlikely error case |
| |
| From: John Fastabend <john.fastabend@gmail.com> |
| |
| [ Upstream commit 7e6b27a69167f97c56b5437871d29e9722c3e470 ] |
| |
| If skb_linearize is needed and fails we could leak a msg on the error |
| handling. To fix ensure we kfree the msg block before returning error. |
| Found during code review. |
| |
| Fixes: 4363023d2668e ("bpf, sockmap: Avoid failures from skb_to_sgvec when skb has frag_list") |
| Signed-off-by: John Fastabend <john.fastabend@gmail.com> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Reviewed-by: Cong Wang <cong.wang@bytedance.com> |
| Link: https://lore.kernel.org/bpf/20210712195546.423990-2-john.fastabend@gmail.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/core/skmsg.c | 16 +++++++++++----- |
| 1 file changed, 11 insertions(+), 5 deletions(-) |
| |
| diff --git a/net/core/skmsg.c b/net/core/skmsg.c |
| index 539c83a45665..b2410a1bfa23 100644 |
| --- a/net/core/skmsg.c |
| +++ b/net/core/skmsg.c |
| @@ -531,10 +531,8 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb, |
| if (skb_linearize(skb)) |
| return -EAGAIN; |
| num_sge = skb_to_sgvec(skb, msg->sg.data, 0, skb->len); |
| - if (unlikely(num_sge < 0)) { |
| - kfree(msg); |
| + if (unlikely(num_sge < 0)) |
| return num_sge; |
| - } |
| |
| copied = skb->len; |
| msg->sg.start = 0; |
| @@ -553,6 +551,7 @@ static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb) |
| { |
| struct sock *sk = psock->sk; |
| struct sk_msg *msg; |
| + int err; |
| |
| /* If we are receiving on the same sock skb->sk is already assigned, |
| * skip memory accounting and owner transition seeing it already set |
| @@ -571,7 +570,10 @@ static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb) |
| * into user buffers. |
| */ |
| skb_set_owner_r(skb, sk); |
| - return sk_psock_skb_ingress_enqueue(skb, psock, sk, msg); |
| + err = sk_psock_skb_ingress_enqueue(skb, psock, sk, msg); |
| + if (err < 0) |
| + kfree(msg); |
| + return err; |
| } |
| |
| /* Puts an skb on the ingress queue of the socket already assigned to the |
| @@ -582,12 +584,16 @@ static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb |
| { |
| struct sk_msg *msg = kzalloc(sizeof(*msg), __GFP_NOWARN | GFP_ATOMIC); |
| struct sock *sk = psock->sk; |
| + int err; |
| |
| if (unlikely(!msg)) |
| return -EAGAIN; |
| sk_msg_init(msg); |
| skb_set_owner_r(skb, sk); |
| - return sk_psock_skb_ingress_enqueue(skb, psock, sk, msg); |
| + err = sk_psock_skb_ingress_enqueue(skb, psock, sk, msg); |
| + if (err < 0) |
| + kfree(msg); |
| + return err; |
| } |
| |
| static int sk_psock_handle_skb(struct sk_psock *psock, struct sk_buff *skb, |
| -- |
| 2.30.2 |
| |