netdev - Re: [PATCH bpf-next v1 1/2] ktls, sockmap: Fix missing uncharge operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <048b968d1993ca84e3442da936bc8e4be07d98f4@linux.dev>
Date: Tue, 29 Apr 2025 01:55:53 +0000
From: "Jiayuan Chen" <jiayuan.chen@...ux.dev>
To: "Cong Wang" <xiyou.wangcong@...il.com>
Cc: bpf@...r.kernel.org, mrpre@....com, "Boris Pismenny" <borisp@...dia.com>,
 "John Fastabend" <john.fastabend@...il.com>, "Jakub Kicinski"
 <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, "Paolo Abeni" <pabeni@...hat.com>, "Simon
 Horman" <horms@...nel.org>, "Alexei Starovoitov" <ast@...nel.org>,
 "Daniel Borkmann" <daniel@...earbox.net>, "Andrii Nakryiko"
 <andrii@...nel.org>, "Martin KaFai Lau" <martin.lau@...ux.dev>, "Eduard
 Zingerman" <eddyz87@...il.com>, "Song Liu" <song@...nel.org>, "Yonghong
 Song" <yonghong.song@...ux.dev>, "KP Singh" <kpsingh@...nel.org>,
 "Stanislav Fomichev" <sdf@...ichev.me>, "Hao Luo" <haoluo@...gle.com>,
 "Jiri Olsa" <jolsa@...nel.org>, "Mykola Lysenko" <mykolal@...com>, "Shuah
 Khan" <shuah@...nel.org>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v1 1/2] ktls, sockmap: Fix missing uncharge
 operation

2025/4/29 08:14, "Cong Wang" <xiyou.wangcong@...il.com> wrote:

> 
> On Fri, Apr 25, 2025 at 01:59:57PM +0800, Jiayuan Chen wrote:
> 
> > 
> > net/tls/tls_sw.c | 7 +++++++
> > 
> >  1 file changed, 7 insertions(+)
> > 
> >  
> > 
> >  diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> > 
> >  index f3d7d19482da..fc88e34b7f33 100644
> > 
> >  --- a/net/tls/tls_sw.c
> > 
> >  +++ b/net/tls/tls_sw.c
> > 
> >  @@ -908,6 +908,13 @@ static int bpf_exec_tx_verdict(struct sk_msg *msg, struct sock *sk,
> > 
> >  &msg_redir, send, flags);
> > 
> >  lock_sock(sk);
> > 
> >  if (err < 0) {
> > 
> >  + /* Regardless of whether the data represented by
> > 
> >  + * msg_redir is sent successfully, we have already
> > 
> >  + * uncharged it via sk_msg_return_zero(). The
> > 
> >  + * msg->sg.size represents the remaining unprocessed
> > 
> >  + * data, which needs to be uncharged here.
> > 
> >  + */
> > 
> >  + sk_mem_uncharge(sk, msg->sg.size);
> > 
> >  *copied -= sk_msg_free_nocharge(sk, &msg_redir);
> > 
> 
> Equivalent to sk_msg_free() ?
> 
> Thanks.
>

Before calling tcp_bpf_sendmsg_redir(), we have already uncharged some
memory using sk_msg_return_zero(). If we perform sk_msg_free(msg_redir),
it will cause the duplicate uncharge of this part of data. If we perform
sk_msg_free(msg), since tcp_bpf_sendmsg_redir() may not have sent any data
and msg->sg.start no longer points to this part of data, it will lead to
memoryleak.

So, directly calling sk_msg_free is not a good idea.