linux-kernel - Re: [PATCH bpf-next v2 0/6] selftests/bpf: Various sockmap-related fixes

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: <87y12xy5fe.fsf@cloudflare.com>
Date: Wed, 09 Oct 2024 11:46:45 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Michal Luczaj <mhal@...x.co>
Cc: Andrii Nakryiko <andrii@...nel.org>,  Eduard Zingerman
 <eddyz87@...il.com>,  Mykola Lysenko <mykolal@...com>,  Alexei Starovoitov
 <ast@...nel.org>,  Daniel Borkmann <daniel@...earbox.net>,  Martin KaFai
 Lau <martin.lau@...ux.dev>,  Song Liu <song@...nel.org>,  Yonghong Song
 <yonghong.song@...ux.dev>,  John Fastabend <john.fastabend@...il.com>,  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>,  bpf@...r.kernel.org,
  linux-kselftest@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 0/6] selftests/bpf: Various sockmap-related
 fixes

I'm back after a short break. Sorry for delay.

On Wed, Oct 02, 2024 at 10:27 AM +02, Michal Luczaj wrote:
> On 9/27/24 11:15, Jakub Sitnicki wrote:
>> On Fri, Sep 27, 2024 at 12:54 AM +02, Michal Luczaj wrote:
>>> ...
>>> Here's a follow up: my guess is that some checks are missing. I'm not sure
>>> if it's the best approach, but this fixes things for me:
>> 
>> So you have already found a bug with a negative test. Nice.
>> 
>> Your patch makes sense to me.
>
> Great, I'll submit it properly.
>
> Another thing I've noticed is that unsupported (non-TCP) sk_msg redirects
> fail silently, i.e. send() is successful, then packet appears to be
> dropped, but because the BPF_SK_MSG_VERDICT program is never run, the
> verdict[SK_DROP] isn't updated. Is this by design?

That's curious. We don't override the proto::sendmsg callback for
protocols which don't support sk_msg redirects, like UDP:

https://elixir.bootlin.com/linux/v6.12-rc2/source/net/ipv4/udp_bpf.c#L114

The packet should get delivered to the peer socket as w/o sockmap.
I will have to double check that.

> Also, for unsupported af_vsock sk_skb-to-ingress we hit the warning:
>
> [  233.396654] rx_queue is empty, but rx_bytes is non-zero
> [  233.396702] WARNING: CPU: 11 PID: 40601 at net/vmw_vsock/virtio_transport_common.c:589 virtio_transport_stream_dequeue+0x2e5/0x2f0
>
> I'll try to fix that. Now, the series begin to grow long. Should the fixes
> come separately?

Thanks. And yes - if possible, better to push fixes separately. Because
they go through the bpf tree, and they will still land in the upcoming
-rc releases (and get backported).

While improvements go through bpf-next. Of course that sometimes makes
life more difficult if the improvements depend on some fixes...

Not sure if anything from bpf-next gets backported if it has a Fixes
tag. We can ask the stable kernel maintainers, if needed.