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

[Simon Horman]

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?