From: Jakub Sitnicki <jakub@cloudflare.com>
To: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Cc: John Fastabend <john.fastabend@gmail.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Lorenz Bauer <lmb@cloudflare.com>,
Eric Dumazet <edumazet@google.com>,
"David S. Miller" <davem@davemloft.net>,
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
Jakub Kicinski <kuba@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
KP Singh <kpsingh@chromium.org>,
Lingpeng Chen <forrest0579@gmail.com>,
netdev@vger.kernel.org, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org, yuanxzhang@fudan.edu.cn,
kjlu@umn.edu, Xin Tan <tanxin.ctf@gmail.com>
Subject: Re: [PATCH v2] bpf: Fix sk_psock refcnt leak when receiving message
Date: Sun, 26 Apr 2020 13:01:29 +0200 [thread overview]
Message-ID: <87k122v7cm.fsf@cloudflare.com> (raw)
In-Reply-To: <1587872115-42805-1-git-send-email-xiyuyang19@fudan.edu.cn>
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>
next prev parent reply other threads:[~2020-04-26 11:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-04-27 21:21 ` Daniel Borkmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k122v7cm.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=forrest0579@gmail.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kjlu@umn.edu \
--cc=kpsingh@chromium.org \
--cc=kuba@kernel.org \
--cc=kuznet@ms2.inr.ac.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=lmb@cloudflare.com \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=tanxin.ctf@gmail.com \
--cc=xiyuyang19@fudan.edu.cn \
--cc=yhs@fb.com \
--cc=yoshfuji@linux-ipv6.org \
--cc=yuanxzhang@fudan.edu.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.