From: John Fastabend <john.fastabend@gmail.com>
To: Xu Kuohai <xukuohai@huaweicloud.com>,
Martin KaFai Lau <martin.lau@linux.dev>,
Xu Kuohai <xukuohai@huaweicloud.com>,
John Fastabend <john.fastabend@gmail.com>
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org,
Jakub Sitnicki <jakub@cloudflare.com>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH bpf] bpf, sockmap: Fix map type error in sock_map_del_link
Date: Mon, 31 Jul 2023 20:47:49 -0700 [thread overview]
Message-ID: <64c8806537c3a_a427920846@john.notmuch> (raw)
In-Reply-To: <6965ceb9-0b96-ce21-cc72-7d29b42544bd@huaweicloud.com>
Xu Kuohai wrote:
> On 8/1/2023 9:19 AM, Martin KaFai Lau wrote:
> > On 7/28/23 3:56 AM, Xu Kuohai wrote:
> >> sock_map_del_link() operates on both SOCKMAP and SOCKHASH, although
> >> both types have member named "progs", the offset of "progs" member in
> >> these two types is different, so "progs" should be accessed with the
> >> real map type.
> >
> > The patch makes sense to me. Can a test be written to trigger it?
> >
>
> Thank you for the review. I have a messy prog that triggers memleak
> caused by this issue. I'll try to simplify it to a test.
>
> > John, please review.
> >
> >
> > .
>
>
Thanks good catch. One thing I don't see any tests for is deleting a
socket from a sockmap and then trying to use it? My guess is almost
no one deletes sockets from a map except on sock close. Maybe to
reproduce,
1. connect a bunch of sockets to sockhash with verdict prog
2. remove the sockets
3. remove the sockhash
4. that should leak the bpf ref cnt so we could check for the
prog still existing?
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
next prev parent reply other threads:[~2023-08-01 3:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-28 10:56 [PATCH bpf] bpf, sockmap: Fix map type error in sock_map_del_link Xu Kuohai
2023-08-01 1:19 ` Martin KaFai Lau
2023-08-01 2:24 ` Xu Kuohai
2023-08-01 3:47 ` John Fastabend [this message]
2023-08-03 2:47 ` Xu Kuohai
2023-08-05 11:41 ` Jakub Sitnicki
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=64c8806537c3a_a427920846@john.notmuch \
--to=john.fastabend@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jakub@cloudflare.com \
--cc=kuba@kernel.org \
--cc=martin.lau@linux.dev \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=xukuohai@huaweicloud.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox