linux-kernel - Re: [PATCH bpf v5 1/3] bpf: fix wrong copied

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <nch3maxdymvvdq647hijycfj2y242o67wgkch3vksfgrkabtt3@xuskfpo426xz>
Date: Tue, 14 Jan 2025 14:35:34 +0800
From: Jiayuan Chen <mrpre@....com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: bpf@...r.kernel.org, jakub@...udflare.com, john.fastabend@...il.com, 
	netdev@...r.kernel.org, martin.lau@...ux.dev, ast@...nel.org, edumazet@...gle.com, 
	davem@...emloft.net, dsahern@...nel.org, pabeni@...hat.com, 
	linux-kernel@...r.kernel.org, song@...nel.org, andrii@...nel.org, mhal@...x.co, 
	yonghong.song@...ux.dev, daniel@...earbox.net, xiyou.wangcong@...il.com, horms@...nel.org, 
	corbet@....net, eddyz87@...il.com, cong.wang@...edance.com, shuah@...nel.org, 
	mykolal@...com, jolsa@...nel.org, haoluo@...gle.com, sdf@...ichev.me, 
	kpsingh@...nel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH bpf v5 1/3] bpf: fix wrong copied_seq calculation

On Mon, Jan 13, 2025 at 04:04:04PM +0800, Jakub Kicinski wrote:
> On Thu,  9 Jan 2025 17:43:59 +0800 Jiayuan Chen wrote:
> > However, for programs where both stream_parser and stream_verdict are
> > active(strparser purpose), tcp_read_sock() was used instead of
> > tcp_read_skb() (sk_data_ready->strp_data_ready->tcp_read_sock)
> > tcp_read_sock() now still update 'sk->copied_seq', leading to duplicated
> > updates.
> 
> To state the obvious feels like the abstraction between TCP and psock
> has broken down pretty severely at this stage. You're modifying TCP
> and straight up calling TCP functions from skmsg.c :(
>
You are right!

How about we construct code like this:

sk_psock_strp_read_sock(strp)    skmsg.c
    tcp_bpf_strp_read_sock(sk)   tcp_bpf.c
        tcp_read_sock_noack(sk)  tcp.c

In skmsg.c we just register read_sock handler for strparser, then move
core code into tcp_bpf.c. I believe it makes more sense than before as
there already exist some psock with tcp operation(especially ops handler)
implemented in tcp_bpf.c.
 
> > +int tcp_read_sock_noack(struct sock *sk, read_descriptor_t *desc,
> > +			sk_read_actor_t recv_actor, u32 noack,
> > +			u32 *copied_seq)
> > +{
> > +	return __tcp_read_sock(sk, desc, recv_actor,
> > +			       noack, copied_seq);
> > +}
> > +EXPORT_SYMBOL(tcp_read_sock_noack);
> 
> Pretty sure you don't have to export this. skmsg can't be a module.
ok, i will remove it.