macsec_changelink() operates on two netns: dev_net(dev) and the netns of macsec->real_dev. They differ once the macsec device is created in or moved to a netns other than the one holding the real device. The rtnl changelink path checks CAP_NET_ADMIN only against dev_net(dev), so a caller privileged there but not in dev_net(real_dev) can reach real_dev's driver: - IFLA_MACSEC_OFFLOAD -> macsec_update_offload(), which calls ops->mdo_add_secy() / mdo_del_secy() on real_dev. - the tail "propagate to the device" block, which calls ops->mdo_upd_secy() on real_dev for any request against an already offloaded device that does not change the offload mode. This one needs no IFLA_MACSEC_OFFLOAD attribute at all. Gate both with rtnl_dev_link_net_capable(), as the tunnel, geneve, vxlan and macvlan handlers already do. It fires only when the request would reach real_dev: when IFLA_MACSEC_OFFLOAD asks for a mode other than the current one, or when the device is already offloaded. The local attributes of a non-offloaded device stay configurable from its own netns. The check is in the changelink handler rather than in macsec_update_offload(), because the mdo_upd_secy() propagation does not go through that function. Its other caller, macsec_upd_offload() for MACSEC_CMD_UPD_OFFLOAD, is GENL_ADMIN_PERM. The ndo and dellink paths that reach real_dev's driver as a side effect of operating the macsec device itself (open/stop, set_mac_address, dellink) are unchanged here. rtnl_dev_link_net_capable() was added by commit 8165f7ff57d9 ("net: ip_gre: require CAP_NET_ADMIN in the device netns for changelink"); a stable tree without it needs that first. Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure") Cc: # see patch description Cc: Antoine Tenart Assisted-by: LLM Signed-off-by: Aamir Ahmed --- v2: - factor the check out into macsec_changelink_check_netns() (Jakub) - fire the offload term only on an actual transition, so restating the current mode on a non-offloaded device is still allowed (Sashiko, Jakub) - commit message: say why the check stays in the changelink handler (Sashiko) - Cc Antoine Tenart, author of the Fixes: commit (netdev CI) - add the Assisted-by: LLM trailer v1: https://lore.kernel.org/netdev/AS8P251MB000190AAD82CD30EFBC57B4DC8B42@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/ Found and tested with LLM assistance, by auditing rtnl_link_ops changelink handlers for this check. The helper and the changelog were reviewed by hand. Where 8165f7ff57d9 is missing, the call can be open-coded as net_eq(dev_net(dev), dev_net(macsec->real_dev)) || ns_capable(dev_net(macsec->real_dev)->user_ns, CAP_NET_ADMIN) Tested on the base-commit below against three kernels - unpatched, v1 and this v2 - with netdevsim as the offload-capable NIC, and macsec0 (offload mac) and macsec1 (no offload) moved into an unshare -Urn namespace; the netdevsim port and an offloaded macsec2 (row 7) stayed in init_net. ftrace on nsim_macsec_* showed rows 1, 2, 3 and 6 reaching real_dev's driver unpatched (upd_secy, upd_secy, del_secy, add_secy) and no row reaching it on v1 or v2 except rows 7 and 8. Every EPERM carried the new extack. unpatched v1 v2 1 offloaded + unpriv "encrypt off" ok EPERM EPERM 2 offloaded + unpriv "offload mac encrypt off" ok EPERM EPERM 3 offloaded + unpriv "offload off" ok EPERM EPERM 4 !offloaded + unpriv "encrypt off" ok ok ok 5 !offloaded + unpriv "offload off encrypt off" ok EPERM ok 6 !offloaded + unpriv "offload mac" ok EPERM EPERM 7 same-netns privileged "encrypt off" ok ok ok 8 privileged cross-netns "offload off" ok ok ok Rows 1, 2, 3 and 6 unpatched are the bug; row 5 is what v1 wrongly refused. drivers/net/macsec.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 6f9f3acef..e019e7a25 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -3945,6 +3946,33 @@ static int macsec_changelink_common(struct net_device *dev, return 0; } +/* Both a change of the offload state (mdo_add_secy()/mdo_del_secy()) and + * any request against an already offloaded device (mdo_upd_secy()) reach + * the driver of real_dev, which may live in a different network namespace + * from the macsec device. Require CAP_NET_ADMIN over real_dev's namespace + * for those; local attributes of a non-offloaded device are unaffected. + */ +static int macsec_changelink_check_netns(struct net_device *dev, + struct nlattr *data[], + struct netlink_ext_ack *extack) +{ + struct macsec_dev *macsec = macsec_priv(dev); + bool reaches_real_dev; + + reaches_real_dev = macsec_is_offloaded(macsec); + if (data[IFLA_MACSEC_OFFLOAD] && + nla_get_u8(data[IFLA_MACSEC_OFFLOAD]) != macsec->offload) + reaches_real_dev = true; + + if (!reaches_real_dev || + rtnl_dev_link_net_capable(dev, dev_net(macsec->real_dev))) + return 0; + + NL_SET_ERR_MSG(extack, + "Changing a MACsec device whose real device is in another network namespace requires CAP_NET_ADMIN in that namespace"); + return -EPERM; +} + static int macsec_changelink(struct net_device *dev, struct nlattr *tb[], struct nlattr *data[], struct netlink_ext_ack *extack) @@ -3959,6 +3987,10 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[], if (!data) return 0; + ret = macsec_changelink_check_netns(dev, data, extack); + if (ret) + return ret; + if (data[IFLA_MACSEC_CIPHER_SUITE] || data[IFLA_MACSEC_ICV_LEN] || data[IFLA_MACSEC_SCI] || base-commit: 78445023439506ebd83b86d40b1e428a3b309d4a -- 2.43.0