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

["Jiayuan Chen" <jiayuan.chen@linux.dev>]

2025/4/29 08:14, "Cong Wang" <xiyou.wangcong@gmail.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.