From: Michal Luczaj <mhal@rbox.co>
To: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com, shuah@kernel.org
Subject: Re: [PATCH net v2 1/2] af_unix: Fix garbage collection of embryos carrying OOB with SCM_RIGHTS
Date: Fri, 17 May 2024 07:59:16 +0200 [thread overview]
Message-ID: <734273bc-2415-43b7-9873-26416aab8900@rbox.co> (raw)
In-Reply-To: <20240517014529.94140-1-kuniyu@amazon.com>
On 5/17/24 03:45, Kuniyuki Iwashima wrote:
> From: Michal Luczaj <mhal@rbox.co>
> Date: Thu, 16 May 2024 16:50:09 +0200
>> GC attempts to explicitly drop oob_skb before purging the hit list.
>
> Sorry for not catching these in v1,
>
> nit: s/oob_skb/oob_skb's reference/
Argh, sorry, I've copy-pasted my own misformulation.
>> The problem is with embryos: kfree_skb(u->oob_skb) is never called on an
>> embryo socket, as those sockets are not directly stacked by the SCC walk.
>
> ", as ..." is not correct and can be just removed. Here we walk
> through embryos as written in the next paragraph but we forget
> dropping oob_skb's refcnt.
Oh, I agree we walk through embryos. I wrote that embryos are not _stacked_
by the SCC walk, i.e. embryos don't appear on the `vertex_stack`. But I
think you're right, such comment of mine would be incorrect anyway. So,
removing and resending.
>> The python script below [0] sends a listener's fd to its embryo as OOB
>> data. While GC does collect the embryo's queue, it fails to drop the OOB
>> skb's refcount. The skb which was in embryo's receive queue stays as
>> unix_sk(sk)->oob_skb and keeps the listener's refcount [1].
>>
>> Tell GC to dispose embryo's oob_skb.
>>
>> [0]:
>> from array import array
>> from socket import *
>>
>> addr = '\x00unix-oob'
>> lis = socket(AF_UNIX, SOCK_STREAM)
>> lis.bind(addr)
>> lis.listen(1)
>>
>> s = socket(AF_UNIX, SOCK_STREAM)
>> s.connect(addr)
>> scm = (SOL_SOCKET, SCM_RIGHTS, array('i', [lis.fileno()]))
>> s.sendmsg([b'x'], [scm], MSG_OOB)
>> lis.close()
>>
>> [1]
>> $ grep unix-oob /proc/net/unix
>> $ ./unix-oob.py
>> $ grep unix-oob /proc/net/unix
>> 0000000000000000: 00000002 00000000 00000000 0001 02 0 @unix-oob
>> 0000000000000000: 00000002 00000000 00010000 0001 01 6072 @unix-oob
>>
>> Fixes: 4090fa373f0e ("af_unix: Replace garbage collection algorithm.")
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>
> with the above corrected, you can add
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> Thanks!
All right, thank you.
One question: git send-email automatically adds my Signed-off-by to your
patch (patch 2/2 in this series). Should I leave it that way?
>> ...
next prev parent reply other threads:[~2024-05-17 5:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-16 14:50 [PATCH net v2 0/2] af_unix: Fix GC and improve selftest Michal Luczaj
2024-05-16 14:50 ` [PATCH net v2 1/2] af_unix: Fix garbage collection of embryos carrying OOB with SCM_RIGHTS Michal Luczaj
2024-05-17 1:45 ` Kuniyuki Iwashima
2024-05-17 5:59 ` Michal Luczaj [this message]
2024-05-17 7:47 ` Kuniyuki Iwashima
2024-05-17 8:55 ` Michal Luczaj
2024-05-17 9:22 ` Kuniyuki Iwashima
2024-05-17 9:58 ` Michal Luczaj
2024-05-17 19:24 ` Jakub Kicinski
2024-05-19 8:47 ` Michal Luczaj
2024-05-16 14:50 ` [PATCH net v2 2/2] selftest: af_unix: Make SCM_RIGHTS into OOB data Michal Luczaj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=734273bc-2415-43b7-9873-26416aab8900@rbox.co \
--to=mhal@rbox.co \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=kuniyu@amazon.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.