* [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message
@ 2020-04-26 3:35 Xiyu Yang
2020-04-26 11:01 ` Jakub Sitnicki
2020-04-27 21:21 ` Daniel Borkmann
0 siblings, 2 replies; 3+ messages in thread
From: Xiyu Yang @ 2020-04-26 3:35 UTC (permalink / raw)
To: John Fastabend, Daniel Borkmann, Jakub Sitnicki, Lorenz Bauer,
Eric Dumazet, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Jakub Kicinski, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
KP Singh, Lingpeng Chen, netdev, bpf, linux-kernel
Cc: yuanxzhang, kjlu, Xiyu Yang, Xin Tan
tcp_bpf_recvmsg() invokes sk_psock_get(), which returns a reference of
the specified sk_psock object to "psock" with increased refcnt.
When tcp_bpf_recvmsg() returns, local variable "psock" becomes invalid,
so the refcount should be decreased to keep refcount balanced.
The reference counting issue happens in several exception handling paths
of tcp_bpf_recvmsg(). When those error scenarios occur such as "flags"
includes MSG_ERRQUEUE, the function forgets to decrease the refcnt
increased by sk_psock_get(), causing a refcnt leak.
Fix this issue by calling sk_psock_put() or pulling up the error queue
read handling when those error scenarios occur.
Fixes: e7a5f1f1cd000 ("bpf/sockmap: Read psock ingress_msg before sk_receive_queue")
Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
Changes in v2:
- Add Fixes tag
- Pull up the error queue read handling
---
net/ipv4/tcp_bpf.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 5a05327f97c1..ff96466ea6da 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -262,14 +262,17 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
struct sk_psock *psock;
int copied, ret;
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return inet_recv_error(sk, msg, len, addr_len);
+
psock = sk_psock_get(sk);
if (unlikely(!psock))
return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
- if (unlikely(flags & MSG_ERRQUEUE))
- return inet_recv_error(sk, msg, len, addr_len);
if (!skb_queue_empty(&sk->sk_receive_queue) &&
- sk_psock_queue_empty(psock))
+ sk_psock_queue_empty(psock)) {
+ sk_psock_put(sk, psock);
return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ }
lock_sock(sk);
msg_bytes_ready:
copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message
2020-04-26 3:35 [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message Xiyu Yang
@ 2020-04-26 11:01 ` Jakub Sitnicki
2020-04-27 21:21 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Sitnicki @ 2020-04-26 11:01 UTC (permalink / raw)
To: Xiyu Yang
Cc: John Fastabend, Daniel Borkmann, Lorenz Bauer, Eric Dumazet,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, KP Singh, Lingpeng Chen, netdev,
bpf, linux-kernel, yuanxzhang, kjlu, Xin Tan
On Sun, Apr 26, 2020 at 05:35 AM CEST, Xiyu Yang wrote:
> tcp_bpf_recvmsg() invokes sk_psock_get(), which returns a reference of
> the specified sk_psock object to "psock" with increased refcnt.
>
> When tcp_bpf_recvmsg() returns, local variable "psock" becomes invalid,
> so the refcount should be decreased to keep refcount balanced.
>
> The reference counting issue happens in several exception handling paths
> of tcp_bpf_recvmsg(). When those error scenarios occur such as "flags"
> includes MSG_ERRQUEUE, the function forgets to decrease the refcnt
> increased by sk_psock_get(), causing a refcnt leak.
>
> Fix this issue by calling sk_psock_put() or pulling up the error queue
> read handling when those error scenarios occur.
>
> Fixes: e7a5f1f1cd000 ("bpf/sockmap: Read psock ingress_msg before sk_receive_queue")
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> ---
> Changes in v2:
> - Add Fixes tag
> - Pull up the error queue read handling
> ---
> net/ipv4/tcp_bpf.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 5a05327f97c1..ff96466ea6da 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -262,14 +262,17 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> struct sk_psock *psock;
> int copied, ret;
>
> + if (unlikely(flags & MSG_ERRQUEUE))
> + return inet_recv_error(sk, msg, len, addr_len);
> +
> psock = sk_psock_get(sk);
> if (unlikely(!psock))
> return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
> - if (unlikely(flags & MSG_ERRQUEUE))
> - return inet_recv_error(sk, msg, len, addr_len);
> if (!skb_queue_empty(&sk->sk_receive_queue) &&
> - sk_psock_queue_empty(psock))
> + sk_psock_queue_empty(psock)) {
> + sk_psock_put(sk, psock);
> return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
> + }
> lock_sock(sk);
> msg_bytes_ready:
> copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags);
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message
2020-04-26 3:35 [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message Xiyu Yang
2020-04-26 11:01 ` Jakub Sitnicki
@ 2020-04-27 21:21 ` Daniel Borkmann
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2020-04-27 21:21 UTC (permalink / raw)
To: Xiyu Yang, John Fastabend, Jakub Sitnicki, Lorenz Bauer,
Eric Dumazet, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Jakub Kicinski, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
KP Singh, Lingpeng Chen, netdev, bpf, linux-kernel
Cc: yuanxzhang, kjlu, Xin Tan
On 4/26/20 5:35 AM, Xiyu Yang wrote:
> tcp_bpf_recvmsg() invokes sk_psock_get(), which returns a reference of
> the specified sk_psock object to "psock" with increased refcnt.
>
> When tcp_bpf_recvmsg() returns, local variable "psock" becomes invalid,
> so the refcount should be decreased to keep refcount balanced.
>
> The reference counting issue happens in several exception handling paths
> of tcp_bpf_recvmsg(). When those error scenarios occur such as "flags"
> includes MSG_ERRQUEUE, the function forgets to decrease the refcnt
> increased by sk_psock_get(), causing a refcnt leak.
>
> Fix this issue by calling sk_psock_put() or pulling up the error queue
> read handling when those error scenarios occur.
>
> Fixes: e7a5f1f1cd000 ("bpf/sockmap: Read psock ingress_msg before sk_receive_queue")
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-27 21:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-26 3:35 [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message Xiyu Yang
2020-04-26 11:01 ` Jakub Sitnicki
2020-04-27 21:21 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).