Re: [PATCH net v3] gtp: Require 'CAP_NET_ADMIN' when joining the gtp multicast group

[Anil Kaushik]

On Thu, Oct 29, 2026 at 9:38 AM <netdev-bot+sashiko@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] An unprivileged local user can still receive gtp multicast
>   notifications (PDP add/delete and echo responses) by joining the…
>
> --
>
> gtp: Require 'CAP_NET_ADMIN' when joining the gtp multicast group
>
> This patch marks the gtp generic netlink multicast group with
> GENL_MCAST_CAP_NET_ADMIN in drivers/net/gtp.c and with uns-admin-perm in
> the gtp netlink spec. The goal is to stop unprivileged users from
> receiving the PDP context add/delete and echo response notifications,
> which carry the same data as a GETPDP reply.
>

Yes, this works, but the gap is in the genetlink core, not in gtp.
genl_unregister_mc_groups() clears the members of the IDs it frees, but
nothing clears them when free IDs are given to a new family. So it hits
every modular group with a GENL_MCAST_CAP_* flag, psample and
drop_monitor too.

This patch is still needed, and it does not change. I will send a
separate fix to net that clears the members of the new IDs in
genl_validate_assign_mc_groups(), like the unregister path does.

> [ ... ]
>
> > diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> > index 86c8c88b702cf..1dd32e979cf59 100644
> > --- a/drivers/net/gtp.c
> > +++ b/drivers/net/gtp.c
> > @@ -118,7 +118,8 @@ enum gtp_multicast_groups {
> >  };
> >
> >  static const struct genl_multicast_group gtp_genl_mcgrps[] = {
> > -     [GTP_GENL_MCGRP] = { .name = GTP_GENL_MCGRP_NAME },
> > +     [GTP_GENL_MCGRP] = { .name = GTP_GENL_MCGRP_NAME,
> > +                          .flags = GENL_MCAST_CAP_NET_ADMIN, },
> >  };
>
> [Severity: High]
> Can an unprivileged user still get these notifications by joining the
> group ID before the gtp family is registered?
>
> The flag is only checked in genl_bind(), and only when a registered
> family owns the requested group. If no family owns it, the loop matches
> nothing and ret stays 0:
>
> net/netlink/genetlink.c:genl_bind() {
>     ...
>         int ret = 0;
>     ...
>         idr_for_each_entry(&genl_fam_idr, family, id) {
>     ...
>                 i = group - family->mcgrp_offset;
>                 if (i < 0 || i >= family->n_mcgrps)
>                         continue;
>     ...
>         }
>
>         up_read(&cb_lock);
>         return ret;
> }
>
> The genl socket is NL_CFG_F_NONROOT_RECV. For NETLINK_ADD_MEMBERSHIP,
> netlink_setsockopt() only checks the ID against nlk->ngroups. So an
> unprivileged NETLINK_GENERIC socket can join any free ID that fits in
> the current bitmap.
>
> When gtp is loaded later, the call chain is
> genl_register_family()->genl_validate_assign_mc_groups()->
> genl_allocate_reserve_groups(), which gives gtp the first free ID. After
> that, only __netlink_change_ngroups() runs to resize the table. Sockets
> already subscribed to that ID are not cleared or rechecked.
>
> do_one_broadcast() only looks at the socket's group bitmap and netns.
> So the earlier subscriber would receive what gtp_tunnel_notify() and the
> echo response path send through genlmsg_multicast_netns(...,
> GTP_GENL_MCGRP, ...).
>
> The same user can also trigger the module load. CTRL_CMD_GETFAMILY does
> not need GENL_ADMIN_PERM. ctrl_getfamily() calls request_module() for
> "net-pf-16-proto-16-family-gtp", and gtp.c declares
> MODULE_ALIAS_GENL_FAMILY("gtp").
>
> The commit message says:
>
>     This fix blocks the unprivileged users.
>
> That claim fails when gtp is built as a module and is not yet loaded
> when the user subscribes.
>
> Could this also be closed in the genetlink core? One option is for
> genl_validate_assign_mc_groups() to clear existing members of newly
> allocated IDs with __netlink_clear_multicast_users(). Another is for
> genl_bind() to reject IDs that no family owns. Either change would go on
> top of this patch, not replace it.
>
> The same gap seems to affect other modular groups that use the
> GENL_MCAST_CAP_* flags, such as the psample one.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027130623.3136658-1-anilkaushikwireless%40gmail.com