[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <8f04fd2cec5faaad04efb29f9c8ada7057d7d90f4ee76114b5b1095c8e0fc81e@mail.kernel.org>
Date: Fri, 9 Jan 2026 10:22:19 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: qikeyu2017@...il.com,bpf@...r.kernel.org
Cc: linux-kernel@...r.kernel.org,bjorn@...nel.org,hawk@...nel.org,pabeni@...hat.com,magnus.karlsson@...el.com,daniel@...earbox.net,maciej.fijalkowski@...el.com,kuba@...nel.org,edumazet@...gle.com,horms@...nel.org,ast@...nel.org,sdf@...ichev.me,john.fastabend@...il.com,qikeyu2017@...il.com,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf] xsk: fix init race causing NPD/UAF in xsk_create()
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f093c3453f64..58e9c61c29e0 100644
[ ... ]
> @@ -1922,10 +1923,6 @@ static int __init xsk_init(void)
> if (err)
> goto out;
>
> - err = sock_register(&xsk_family_ops);
> - if (err)
> - goto out_proto;
> -
> err = register_pernet_subsys(&xsk_net_ops);
> if (err)
> goto out_sk;
[ ... ]
> @@ -1942,16 +1939,21 @@ static int __init xsk_init(void)
> goto out_unreg_notif;
> }
>
> + err = sock_register(&xsk_family_ops);
> + if (err)
> + goto out_proto;
> +
> return 0;
>
> +out_proto:
> + proto_unregister(&xsk_proto);
> + kmem_cache_destroy(xsk_tx_generic_cache);
> out_unreg_notif:
> unregister_netdevice_notifier(&xsk_netdev_notifier);
> out_pernet:
> unregister_pernet_subsys(&xsk_net_ops);
> out_sk:
> sock_unregister(PF_XDP);
^^^^^^^^^^^^^^^^
With sock_register() now at the end of xsk_init(), the out_sk label is
reached via goto from register_pernet_subsys(), register_netdevice_notifier(),
or kmem_cache_create() failures - all of which occur before sock_register()
is called.
Shouldn't out_sk be removed entirely, and the gotos redirected? For example,
if register_pernet_subsys() fails, we goto out_sk and call sock_unregister()
on a family that was never registered. The same applies to the other error
paths that fall through to out_sk.
While sock_unregister() on an unregistered family appears to be benign (it
just sets NULL to NULL), it will print "NET: Unregistered PF_XDP protocol
family" to the kernel log even though it was never registered in the first
place.
> -out_proto:
> - proto_unregister(&xsk_proto);
> out:
> return err;
> }
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20848468294
Powered by blists - more mailing lists