netdev - RE: [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf

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: <21313b79ef0c4059a0d9cfa3fd8746fd@huawei.com>
Date:   Tue, 19 Oct 2021 03:03:41 +0000
From:   "liujian (CE)" <liujian56@...wei.com>
To:     John Fastabend <john.fastabend@...il.com>,
        Jakub Sitnicki <jakub@...udflare.com>
CC:     "daniel@...earbox.net" <daniel@...earbox.net>,
        "lmb@...udflare.com" <lmb@...udflare.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org>,
        "dsahern@...nel.org" <dsahern@...nel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "kafai@...com" <kafai@...com>,
        "songliubraving@...com" <songliubraving@...com>,
        "yhs@...com" <yhs@...com>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: RE: [PATCH] bpf, sockmap: Do not read sk_receive_queue in
 tcp_bpf_recvmsg if strparser enabled



> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@...il.com]
> Sent: Monday, October 18, 2021 9:54 PM
> To: Jakub Sitnicki <jakub@...udflare.com>; liujian (CE)
> <liujian56@...wei.com>
> Cc: john.fastabend@...il.com; daniel@...earbox.net; lmb@...udflare.com;
> edumazet@...gle.com; davem@...emloft.net; kuba@...nel.org;
> yoshfuji@...ux-ipv6.org; dsahern@...nel.org; ast@...nel.org;
> andrii@...nel.org; kafai@...com; songliubraving@...com; yhs@...com;
> kpsingh@...nel.org; netdev@...r.kernel.org; bpf@...r.kernel.org
> Subject: Re: [PATCH] bpf, sockmap: Do not read sk_receive_queue in
> tcp_bpf_recvmsg if strparser enabled
> 
> Jakub Sitnicki wrote:
> > On Fri, Oct 15, 2021 at 10:01 AM CEST, Liu Jian wrote:
> > > If the strparser function of sk is turned on, all received data
> > > needs to be processed by strparser first.
> > >
> > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg
> > > interface")
> > > Signed-off-by: Liu Jian <liujian56@...wei.com>
> > > ---
> >
> > [...]
> >
> > >  net/core/skmsg.c      | 5 +++++
> > >  net/ipv4/tcp_bpf.c    | 9 ++++++---
> > >  3 files changed, 17 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h index
> > > 94e2a1f6e58d..25e92dff04aa 100644
> > > --- a/include/linux/skmsg.h
> > > +++ b/include/linux/skmsg.h
> > > @@ -390,6 +390,7 @@ void sk_psock_stop(struct sk_psock *psock, bool
> > > wait);  int sk_psock_init_strp(struct sock *sk, struct sk_psock
> > > *psock);  void sk_psock_start_strp(struct sock *sk, struct sk_psock
> > > *psock);  void sk_psock_stop_strp(struct sock *sk, struct sk_psock
> > > *psock);
> > > +bool sk_psock_strparser_started(struct sock *sk);
> > >  #else
> > >  static inline int sk_psock_init_strp(struct sock *sk, struct
> > > sk_psock *psock)  { @@ -403,6 +404,11 @@ static inline void
> > > sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)  static
> > > inline void sk_psock_stop_strp(struct sock *sk, struct sk_psock
> > > *psock)  {  }
> > > +
> > > +static inline bool sk_psock_strparser_started(struct sock *sk) {
> > > +	return false;
> > > +}
> > >  #endif
> > >
> > >  void sk_psock_start_verdict(struct sock *sk, struct sk_psock
> > > *psock); diff --git a/net/core/skmsg.c b/net/core/skmsg.c index
> > > e85b7f8491b9..dd64ef854f3e 100644
> > > --- a/net/core/skmsg.c
> > > +++ b/net/core/skmsg.c
> > > @@ -1105,6 +1105,11 @@ void sk_psock_start_strp(struct sock *sk,
> struct sk_psock *psock)
> > >  	sk->sk_write_space = sk_psock_write_space;  }
> > >
> > > +bool sk_psock_strparser_started(struct sock *sk) {
> > > +	return sk->sk_data_ready == sk_psock_strp_data_ready;
> >
> > What if kTLS is configured on the socket? I think this check won't work then.
> 
> Liu, did you see this. I think its a bit cleaner, avoids the extra parser check in
> hotpath, and should solve the issue?
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20211011191647.418
> 704-3-john.fastabend@...il.com/
> 
> I think it should also address Jakub's concern.
> 
I am sorry, I did not see the patch before.
I think it can solve my issue, please ignore my patch.
Thanks.

> Thanks,
> John
> 
> >
> > > +}
> > > +
> > >  void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)  {
> > >  	if (!psock->saved_data_ready)
> >
> > [...]
>