Re: [PATCH net 1/1] udp: diag: bound bucket lock hold time

[zihan xi]

On Sat, Sep 5, 2026 at 11:12 PM Simon Horman <horms@xxxxxxxxxx> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/#/patchset/133b6aee9e2c908c9da37d5585b3d2cd016906cd.1788187473.git.zihanx@xxxxxxxxxx?part=1
> ---
> udp: diag: bound bucket lock hold time
>
> This patch limits the time the UDP hash bucket spinlock is held during
> netlink socket dumps by collecting a bounded batch of matching sockets.
> It takes a reference to each socket, releases the lock, processes the
> batch, and resumes from a preserved cursor.
>
> > diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
> > index f4b24e628cf8d..18cfe0df654dd 100644
> > --- a/net/ipv4/udp_diag.c
> > +++ b/net/ipv4/udp_diag.c
> [ ... ]
> > +static void udp_diag_dump_done(struct netlink_callback *cb)
> > +{
> > +     struct inet_diag_dump_data *cb_data = cb->data;
> > +     struct sock *sk = (struct sock *)cb->args[2];
> > +
> > +     if (sk) {
> > +             cb->args[2] = 0;
> > +             sock_put(sk);
> > +     }
> > +     cb_data->dump_done = NULL;
> > +     module_put(THIS_MODULE);
> > +}
>
> [Severity: High]
> Does this module_put() introduce a race condition with module unloading?
>
> When an active netlink dump for UDP sockets finishes, inet_diag_dump_done()
> invokes udp_diag_dump_done(). This function drops its module reference using
> module_put(THIS_MODULE) as its final step.
>
> If this was the last reference, a concurrent rmmod udp_diag operation can
> proceed and unmap the module's text segment before the thread executing
> udp_diag_dump_done() executes its return instruction to return to inet_diag.
>
> Could this result in a kernel panic due to the CPU attempting to execute
> unmapped memory?

Yes.  If that module_put() drops the last reference, rmmod can unmap
udp_diag while dump_done() is still returning to inet_diag.

I will keep the extra module_get() in udp_diag_dump(), and move the
matching module_put() into inet_diag_dump_done() so it runs after the
callback has returned.

v2 incoming.