Re: [PATCH net] net: pin protocol module before socket allocation

[Kuniyuki Iwashima]

On Sun, Aug 23, 2026 at 10:13 AM Chengfeng Ye <nicoyip.dev@xxxxxxxxx> wrote:
>
> sk_prot_alloc() reads prot->slab and starts allocating the socket
> before it gets a reference to prot->owner. A protocol module can begin
> unloading after its protocol was selected but before the reference is
> taken, allowing this interleaving:
>
>   CPU 0                                CPU 1
>   slab = prot->slab
>                                        proto_unregister(prot)
>                                          kmem_cache_destroy(prot->slab)
>   kmem_cache_alloc(slab, ...)
>
> kmem_cache_alloc() then dereferences a freed struct kmem_cache and can
> crash or corrupt memory. The kernel reported:
>
>   Oops: general protection fault, probably for non-canonical address
>   KASAN: maybe wild-memory-access in range
>   RIP: kmem_cache_alloc_noprof+0x63/0x370
>   Call Trace:
>    sk_prot_alloc+0x74/0x2c0
>    sk_alloc+0x2b/0x6c0
>    inet_create+0x2cd/0xd40

inet_create() rather looks broken.

inet_unregister_protosw() calls synchronize_net(), but the following
proto_unregister() might be done after rcu_read_unlock() and before
WARN_ON(!answer_prot->slab) in inet_create().

Which module did you use to trigger the bug ?


>    __sock_create+0x1c3/0x430
>    __sys_socket+0x116/0x1d0
>
> Take the module reference before reading prot->slab so module removal
> cannot destroy the cache during allocation. Drop that reference after
> freeing the allocation on either failure path; on success sk_prot_free()
> continues to release it as before.
>
> Fixes: a79af59efd20 ("[NET]: Fix module reference counts for loadable protocol modules")
> Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> ---
>  net/core/sock.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 1ad41904db25..59f4b15fd594 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2240,33 +2240,34 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
>         struct sock *sk;
>         struct kmem_cache *slab;
>
> +       if (!try_module_get(prot->owner))
> +               return NULL;
> +
>         slab = prot->slab;
>         if (slab != NULL) {
>                 sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
>                 if (!sk)
> -                       return sk;
> +                       goto out_module_put;
>                 if (want_init_on_alloc(priority))
>                         sk_prot_clear_nulls(sk, prot->obj_size);
>         } else
>                 sk = kmalloc(prot->obj_size, priority);
>
> -       if (sk != NULL) {
> -               if (security_sk_alloc(sk, family, priority))
> -                       goto out_free;
> -
> -               if (!try_module_get(prot->owner))
> -                       goto out_free_sec;
> -       }
> +       if (!sk)
> +               goto out_module_put;
> +
> +       if (security_sk_alloc(sk, family, priority))
> +               goto out_free;
>
>         return sk;
>
> -out_free_sec:
> -       security_sk_free(sk);
>  out_free:
>         if (slab != NULL)
>                 kmem_cache_free(slab, sk);
>         else
>                 kfree(sk);
> +out_module_put:
> +       module_put(prot->owner);
>         return NULL;
>  }
>
> --
> 2.43.0