mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4 0/2] genetlink: fix joining multicast groups no family owns
@ 2026-11-02 15:58 Anil Kaushik
  2026-11-02 15:58 ` [PATCH net v4 1/2] genetlink: Reject " Anil Kaushik
  2026-11-02 15:58 ` [PATCH net v4 2/2] netlink: Check a group again if it was released while joining it Anil Kaushik
  0 siblings, 2 replies; 3+ messages in thread
From: Anil Kaushik @ 2026-11-02 15:58 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Ido Schimmel, Nicolas Dichtel, Jacob Keller,
	Jiri Pirko, Florian Westphal, Mat Martineau, XingWang Xiang,
	Alice Ryhl, linux-kernel, Anil Kaushik

genl_bind() checks the GENL_MCAST_CAP_* flags of a group, but the
membership bit is set later, after the lock is dropped. A socket can end
up a member of a group whose flags were never checked for it: when it
joins a group ID that no family owns, or when the family of the group is
unregistered between the check and the point where the bit is set.

Patch 1 rejects joins to group IDs that no family owns. Patch 2 counts,
per group, how many times its members were cleared, which genetlink does
when a family is unregistered. If the count changed after the check, the
group is checked again against its current owner, after the bind
callback is undone if that owner is the one it reached. The core cannot
always tell, see patch 2.

Tested as an unprivileged user, with an msleep() added after the bind
callback in netlink_setsockopt() and netlink_bind(), and a test family
that registers again with GENL_MCAST_CAP_NET_ADMIN:

                                         patch 1 only   v4
  setsockopt:
  unregister in the window,
  register later                         member         not member
  unregister and register in the window  member         not member
  another family unregistered
  (must still join)                      member         member
  bind():
  unregister and register in the window  member         not member

With the msleep() before the bind callback instead, and a test family
that registers again without the flag, the family is bound once, and not
twice as with v3:

                                         v3             v4
  binds while a member, setsockopt/bind  2              1
  binds once the socket is closed        1              0

The test family counts its bind and unbind callbacks: they stay
balanced in all these cases. If the family is replaced just after the
bind callback returned, and before the second read, with an msleep()
there, it gets one unbind too many. The socket is still rejected or
checked against the new family. Normal joins and rtnetlink bind() show
no change.

The test from v1 (join all free IDs, then load gtp and drop_monitor)
gives the same result as before.

I used an LLM to help write and review the code, the tests and the
commit messages of this series. I checked and tested every change
myself.

Changes in v4:
- Patch 2: read the count after ->netlink_bind() too, and call
  ->netlink_unbind() before checking again if the count had already
  changed: ->netlink_bind() can have reached the new owner, which was
  then bound twice (netdev Sashiko). The commit message says what the
  core still cannot tell.
- Patch 1: no change. It does not close the window on its own, patch 2
  does, and it keeps its tags (netdev Sashiko asked about the order).
- The deadlock netdev Sashiko found on the insert failure path of
  netlink_bind() exists without this series, it has its own patch:
  https://lore.kernel.org/netdev/20261102120929.4077030-1-anilkaushikwireless@gmail.com/

Changes in v3:
- Patch 2: check the group again in af_netlink.c instead of clearing
  the members when a family registers. This also covers the case where
  the family is unregistered and another one registers, both in the
  window, does not walk the sockets on every registration, and does not
  touch the members of a group another family still owns (Sashiko).
- Fixes: tag of both patches: the race exists since genl_bind() first
  checked a capability, in 4d54cc32112d (Sashiko).

Changes in v2:
- New patch 2 (Sashiko).
- Patch 1: no change, added Reviewed-by tags.

v3: https://lore.kernel.org/netdev/20261101101347.3910654-1-anilkaushikwireless@gmail.com/
v2: https://lore.kernel.org/netdev/20261031054632.3697295-1-anilkaushikwireless@gmail.com/
v1: https://lore.kernel.org/netdev/20261029062214.3407223-1-anilkaushikwireless@gmail.com/

Anil Kaushik (2):
  genetlink: Reject joining multicast groups no family owns
  netlink: Check a group again if it was released while joining it

 net/netlink/af_netlink.c | 79 +++++++++++++++++++++++++++++++++++++++-
 net/netlink/af_netlink.h |  1 +
 net/netlink/genetlink.c  |  3 +-
 3 files changed, 81 insertions(+), 2 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-02 15:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 15:58 [PATCH net v4 0/2] genetlink: fix joining multicast groups no family owns Anil Kaushik
2026-11-02 15:58 ` [PATCH net v4 1/2] genetlink: Reject " Anil Kaushik
2026-11-02 15:58 ` [PATCH net v4 2/2] netlink: Check a group again if it was released while joining it Anil Kaushik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®