On 6/27/24 12:40, Jakub Sitnicki wrote:
> On Wed, Jun 26, 2024 at 12:19 PM +02, Michal Luczaj wrote:
>> On 6/24/24 16:15, Jakub Sitnicki wrote:
>>> On Sun, Jun 23, 2024 at 12:25 AM +02, Michal Luczaj wrote:
>>>> 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 <kuniyu@xxxxxxxxxx>
>>>> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
>>>> Signed-off-by: Michal Luczaj <mhal@xxxxxxx>
>>>> ---
>>>
>>> [+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://urldefense.com/v3/__https://lore.kernel.org/r/4bac0a8a-eeaa-48ef-aeba-2a6e73c0b982@rbox.co__;!!ACWV5N9M2RV99hQ!INUzIF25cVLOogl5HVn1FemXyw-iTBF358Wi77LDaYGg2UY3mi7Q1sTnZiUhkZhEc1qGZgUEGnRhkq4C$
>
> I think we can reach out to Konstantin Ryabitsev at
> konstantin@xxxxxxxxxxxxxxxxxxx. 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.
>
I am fine if the behavior is same as TCP. Thanks a lot for looking into
this.
Regards,
Shoaib
>
>
>
