Re: [PATCH bpf v2] af_unix: Disable MSG_OOB handling for sockets in sockmap/sockhash

On Wed, Jun 26, 2024 at 12:19 PM +02, Michal Luczaj wrote:

On 6/24/24 16:15, Jakub Sitnicki wrote:

quoted

On Sun, Jun 23, 2024 at 12:25 AM +02, Michal Luczaj wrote:

quoted

AF_UNIX socket tracks the most recent OOB packet (in its receive queue)
with an `oob_skb` pointer. BPF redirecting does not account for that: when
an OOB packet is moved between sockets, `oob_skb` is left outdated. This
results in a single skb that may be accessed from two different sockets.

Take the easy way out: silently drop MSG_OOB data targeting any socket that
is in a sockmap or a sockhash. Note that such silent drop is akin to the
fate of redirected skb's scm_fp_list (SCM_RIGHTS, SCM_CREDENTIALS).

For symmetry, forbid MSG_OOB in unix_bpf_recvmsg().

Suggested-by: Kuniyuki Iwashima <redacted>
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Michal Luczaj <redacted>
---

[+CC Cong who authored ->read_skb]

I'm guessing you have a test program that you're developing the fix
against. Would you like to extend the test case for sockmap redirect
from unix stream [1] to incorporate it?

Sadly unix_inet_redir_to_connected needs a fix first because it
hardcodes sotype to SOCK_DGRAM.

Ugh, my last two replies got silently dropped by vger. Is there any way to
tell what went wrong?

Not sure if it was vger or lore archive. Your reply hit my inbox but is
nowhere to be found in the archive:

https://lore.kernel.org/r/4bac0a8a-eeaa-48ef-aeba-2a6e73c0b982@rbox.co

I think we can reach out to Konstantin Ryabitsev at
konstantin@linuxfoundation.org. AFAIK he maintains the lore.kernel.org
archive.

So, again, sure, I'll extend the sockmap redirect test.

Appreciate the help with adding a regression test, if time allows.
Fixes are of course very welcome even without them.

And regarding Rao's comment, I took a look and I think sockmap'ed TCP OOB
does indeed act the same way. I'll try to add that into selftest as well.n

Right, it does sound like we're not clearing the offset kept in
tcp_sock::urg_data when skb is redirected.