Re: [PATCH] io_uring: fix incorrect io_kiocb reference in io_link_skb

[David Kahurani <k.kahurani@gmail.com>]

On Fri, Sep 19, 2025 at 2:23 PM David Kahurani <k.kahurani@gmail.com> wrote:
>
> This is something unrelated but just bringing it up because it is in the same locality.
>
> It doesn't seem like the references(uarg->refcnt) are well accounted for io_notif_data. Any node that gets passed to 'io_tx_ubuf_complete' will gets its refcnt decremented but assuming there's a list of nodes, some of the nodes in the list will not get their reference count decremented and that will trigger the lockdep_assert in 'io_notif_tw_complete'
>
> It doesn't look that this will have any consequences beyond triggering the lockderp_assert, though.
>
> Maybe my analysis is wrong?
>
>
> On Fri, Sep 19, 2025 at 2:16 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> On 9/19/25 10:03, Yang Xiuwei wrote:
>> > From: Yang Xiuwei <yangxiuwei@kylinos.cn>
>> >
>> > In io_link_skb function, there is a bug where prev_notif is incorrectly
>> > assigned using 'nd' instead of 'prev_nd'. This causes the context
>> > validation check to compare the current notification with itself instead
>> > of comparing it with the previous notification.
>> >
>> > Fix by using the correct prev_nd parameter when obtaining prev_notif.
>>
>> Good catch,
>>
>> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
>> Fixes: 6fe4220912d19 ("io_uring/notif: implement notification stacking")
>> Cc: stable@vger.kernel.org
>>
>>
>> > Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
>> >
>> > diff --git a/io_uring/notif.c b/io_uring/notif.c
>> > index 9a6f6e92d742..ea9c0116cec2 100644
>> > --- a/io_uring/notif.c
>> > +++ b/io_uring/notif.c
>> > @@ -85,7 +85,7 @@ static int io_link_skb(struct sk_buff *skb, struct ubuf_info *uarg)
>> >               return -EEXIST;
>> >
>> >       prev_nd = container_of(prev_uarg, struct io_notif_data, uarg);
>> > -     prev_notif = cmd_to_io_kiocb(nd);
>> > +     prev_notif = cmd_to_io_kiocb(prev_nd);
>> >
>> >       /* make sure all noifications can be finished in the same task_work */
>> >       if (unlikely(notif->ctx != prev_notif->ctx ||
>>
>> --
>> Pavel Begunkov
>>
>>

This is something unrelated but just bringing it up because it is in
the same locality.

It doesn't seem like the references(uarg->refcnt) are well accounted
for io_notif_data. Any node that gets passed to 'io_tx_ubuf_complete'
will gets it's refcnt decremented but assuming there's a list of
nodes, some of the nodes in the list will not get their reference
count decremented and that will trigger the lockdep_assert in
'io_notif_tw_complete'

It doesn't look that this will have any consequences beyond triggering
the lockderp_assert, though.