netdev - 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: <20250113160404.7ab0927d@kernel.org>
Date: Mon, 13 Jan 2025 16:04:04 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Jiayuan Chen <mrpre@....com>
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 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 :(

> +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.