linux-kernel - Re: [PATCH bpf-next v1 1/3] bpf, sockmap: Fix incorrect 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: <87tsyo6ets.fsf@cloudflare.com>
Date: Thu, 20 Nov 2025 13:58:23 +0100
From: Jakub Sitnicki <jakub@...udflare.com>
To: "Jiayuan Chen" <jiayuan.chen@...ux.dev>
Cc: bpf@...r.kernel.org,  "John Fastabend" <john.fastabend@...il.com>,
  "David S. Miller" <davem@...emloft.net>,  "Eric Dumazet"
 <edumazet@...gle.com>,  "Jakub Kicinski" <kuba@...nel.org>,  "Paolo Abeni"
 <pabeni@...hat.com>,  "Simon Horman" <horms@...nel.org>,  "Neal Cardwell"
 <ncardwell@...gle.com>,  "Kuniyuki Iwashima" <kuniyu@...gle.com>,  "David
 Ahern" <dsahern@...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>,  "Shuah Khan" <shuah@...nel.org>,
  "Michal Luczaj" <mhal@...x.co>,  "Stefano Garzarella"
 <sgarzare@...hat.com>,  "Cong Wang" <cong.wang@...edance.com>,
  netdev@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v1 1/3] bpf, sockmap: Fix incorrect copied_seq
 calculation

On Thu, Nov 20, 2025 at 02:49 AM GMT, Jiayuan Chen wrote:
> November 20, 2025 at 03:53, "Jakub Sitnicki" <jakub@...udflare.com mailto:jakub@...udflare.com?to=%22Jakub%20Sitnicki%22%20%3Cjakub%40cloudflare.com%3E > wrote:
>
> [...]
>> >  +/* The BPF program sets BPF_F_INGRESS on sk_msg to indicate data needs to be
>> >  + * redirected to the ingress queue of a specified socket. Since BPF_F_INGRESS is
>> >  + * defined in UAPI so that we can't extend this enum for our internal flags. We
>> >  + * define some internal flags here while inheriting BPF_F_INGRESS.
>> >  + */
>> >  +enum {
>> >  + SK_MSG_F_INGRESS = BPF_F_INGRESS, /* (1ULL << 0) */
>> >  + /* internal flag */
>> >  + SK_MSG_F_INGRESS_SELF = (1ULL << 1)
>> >  +};
>> >  +
>> > 
>> I'm wondering if we need additional state to track this.
>> Can we track sk_msg's construted from skb's that were not redirected by
>> setting `sk_msg.sk = sk` to indicate that the source socket is us in
>> sk_psock_skb_ingress_self()?
>
> Functionally, that would work. However, in that case, we would have to hold
> a reference to sk until the sk_msg is read, which would delay the release of
> sk. One concern is that if there is a bug in the read-side application, sk
> might never be released.

We don't need to grab a reference to sk if we're talking about setting
it only in sk_psock_skb_ingress_self(). psock already holds a ref for
psock->sk, and we purge psock->ingress_msg queue when destroying the
psock before releasing the sock ref in sk_psock_destroy().

While there's nothing wrong with an internal flaag, I'm trying to see if
we make things somewhat consistent so as a result sk_msg state is easier
to reason about.

My thinking here is that we already set sk_msg.sk to source socket in
sk_psock_msg_verdict() on sendmsg() path, so we know that this is the
purpose of that field. We could mimic this on recvmsg() path.