netdev - Re: [PATCH bpf v3 4/4] selftest/bpf: Test sockmap redirect for AF_UNIX MSG

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: <c8e083de-0084-4f88-8ee6-f2d4eaab6c8b@rbox.co>
Date: Thu, 11 Jul 2024 22:35:49 +0200
From: Michal Luczaj <mhal@...x.co>
To: Jakub Sitnicki <jakub@...udflare.com>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 john.fastabend@...il.com, kuniyu@...zon.com, Rao.Shoaib@...cle.com,
 cong.wang@...edance.com
Subject: Re: [PATCH bpf v3 4/4] selftest/bpf: Test sockmap redirect for
 AF_UNIX MSG_OOB

On 7/9/24 12:08, Jakub Sitnicki wrote:
> On Sun, Jul 07, 2024 at 11:28 PM +02, Michal Luczaj wrote:
>> Verify that out-of-band packets are silently dropped before they reach the
>> redirection logic. Attempt to recv() stale data that might have been
>> erroneously left reachable from the original socket.
>>
>> The idea is to test with a 2 byte long send(). Should a MSG_OOB flag be in
>> use, only the last byte will be treated as out-of-band. Test fails if
>> verd_mapfd indicates a wrong number of packets processed (e.g. if OOB data
>> wasn't dropped at the source) or if it was still somehow possble to recv()
> 
> Nit: typo s/possble/possible
> 
> Something like below will catch these for you:
> 
> $ cat ~/src/linux/.git/hooks/post-commit
> exec git show --format=email HEAD | ./scripts/checkpatch.pl --strict --codespell

Heh, I have it. Just not in the right repo :) Will fix.

> This AF_UNIX MSG_OOB use case is super exotic, IMO. TBH, I've just
> learned about it. Hence, I think we could use some more comments for the
> future readers.
> 
> Also, it seems like we only need to remove peer1 from sockmap to test
> the behavior. If so, I'd stick to just what is needed to set up the
> test. Extra stuff makes you wonder what was the authors intention.
> 
> I'd also be more direct about checking return value & error. These
> selftests often serve as the only example / API documentation out there.

Yeah, all fair points. Thanks.