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

On Mon, Jul 08, 2024 at 01:10 AM +02, Michal Luczaj wrote:

On 6/27/24 09:40, Jakub Sitnicki wrote:

quoted

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

quoted

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

[...]

quoted

quoted

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.

Yeah, so I also wanted to extend the TCP's redir_to_connected(), but is
that code correct? It seems to be testing REDIR_INGRESS, yet
prog_stream_verdict() doesn't run bpf_sk_redirect_map() with the
BPF_F_INGRESS flag.

Right, we don't have the ingress-to-local [1] setup covered there.

This test case has outgrown its initial coverage purpose - using sockmap
with listening / unconnected sockets - and needs to be split up, IMO.

There are definitely gaps in the coverage of redirect cases, and I'd
like to extend it to cover all combinations (supported and unsupported
ones) [2].

[1] It's a term I coined to make it easier to communiate
    https://github.com/jsitnicki/kubecon-2024-sockmap/blob/main/cheatsheet-sockmap-redirect.png
[2] https://gist.github.com/jsitnicki/578fdd614d181bed2b02922b17972b4e