| Message ID | 20241003104431.12391-1-a.kovaleva@yadro.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1dae9f1187189bc09ff6d25ca97ead711f7e26f9 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3,net] net: Fix an unsafe loop on the list | expand |
From: Anastasia Kovaleva <a.kovaleva@yadro.com> Date: Thu, 3 Oct 2024 13:44:31 +0300 > The kernel may crash when deleting a genetlink family if there are still > listeners for that family: > > Oops: Kernel access of bad area, sig: 11 [#1] > ... > NIP [c000000000c080bc] netlink_update_socket_mc+0x3c/0xc0 > LR [c000000000c0f764] __netlink_clear_multicast_users+0x74/0xc0 > Call Trace: > __netlink_clear_multicast_users+0x74/0xc0 > genl_unregister_family+0xd4/0x2d0 > > Change the unsafe loop on the list to a safe one, because inside the > loop there is an element removal from this list. > > Fixes: b8273570f802 ("genetlink: fix netns vs. netlink table locking (2)") > Cc: stable@vger.kernel.org > Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com> > Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com> > --- > v3: remove trailing "\", change spaces to tab > v2: add CC tag for stable > --- > include/net/sock.h | 2 ++ > net/netlink/af_netlink.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index c58ca8dd561b..db29c39e19a7 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -894,6 +894,8 @@ static inline void sk_add_bind_node(struct sock *sk, > hlist_for_each_entry_safe(__sk, tmp, list, sk_node) > #define sk_for_each_bound(__sk, list) \ > hlist_for_each_entry(__sk, list, sk_bind_node) > +#define sk_for_each_bound_safe(__sk, tmp, list) \ > + hlist_for_each_entry_safe(__sk, tmp, list, sk_bind_node) > > /** > * sk_for_each_entry_offset_rcu - iterate over a list at a given struct offset > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c > index 0b7a89db3ab7..0a9287fadb47 100644 > --- a/net/netlink/af_netlink.c > +++ b/net/netlink/af_netlink.c > @@ -2136,8 +2136,9 @@ void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group) > { > struct sock *sk; > struct netlink_table *tbl = &nl_table[ksk->sk_protocol]; > + struct hlist_node *tmp; > > - sk_for_each_bound(sk, &tbl->mc_list) > + sk_for_each_bound_safe(sk, tmp, &tbl->mc_list) > netlink_update_socket_mc(nlk_sk(sk), group, 0); > } > > -- > 2.40.1
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 3 Oct 2024 13:44:31 +0300 you wrote: > The kernel may crash when deleting a genetlink family if there are still > listeners for that family: > > Oops: Kernel access of bad area, sig: 11 [#1] > ... > NIP [c000000000c080bc] netlink_update_socket_mc+0x3c/0xc0 > LR [c000000000c0f764] __netlink_clear_multicast_users+0x74/0xc0 > Call Trace: > __netlink_clear_multicast_users+0x74/0xc0 > genl_unregister_family+0xd4/0x2d0 > > [...] Here is the summary with links: - [v3,net] net: Fix an unsafe loop on the list https://git.kernel.org/netdev/net/c/1dae9f118718 You are awesome, thank you!
diff --git a/include/net/sock.h b/include/net/sock.h index c58ca8dd561b..db29c39e19a7 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -894,6 +894,8 @@ static inline void sk_add_bind_node(struct sock *sk, hlist_for_each_entry_safe(__sk, tmp, list, sk_node) #define sk_for_each_bound(__sk, list) \ hlist_for_each_entry(__sk, list, sk_bind_node) +#define sk_for_each_bound_safe(__sk, tmp, list) \ + hlist_for_each_entry_safe(__sk, tmp, list, sk_bind_node) /** * sk_for_each_entry_offset_rcu - iterate over a list at a given struct offset diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 0b7a89db3ab7..0a9287fadb47 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2136,8 +2136,9 @@ void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group) { struct sock *sk; struct netlink_table *tbl = &nl_table[ksk->sk_protocol]; + struct hlist_node *tmp; - sk_for_each_bound(sk, &tbl->mc_list) + sk_for_each_bound_safe(sk, tmp, &tbl->mc_list) netlink_update_socket_mc(nlk_sk(sk), group, 0); }
The kernel may crash when deleting a genetlink family if there are still listeners for that family: Oops: Kernel access of bad area, sig: 11 [#1] ... NIP [c000000000c080bc] netlink_update_socket_mc+0x3c/0xc0 LR [c000000000c0f764] __netlink_clear_multicast_users+0x74/0xc0 Call Trace: __netlink_clear_multicast_users+0x74/0xc0 genl_unregister_family+0xd4/0x2d0 Change the unsafe loop on the list to a safe one, because inside the loop there is an element removal from this list. Fixes: b8273570f802 ("genetlink: fix netns vs. netlink table locking (2)") Cc: stable@vger.kernel.org Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com> Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- v3: remove trailing "\", change spaces to tab v2: add CC tag for stable --- include/net/sock.h | 2 ++ net/netlink/af_netlink.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-)