CAP_NET_ADMIN check in tc_ctl_action() makes it not allowed for user ns root

Hi, from commit "net: Allow tc changes in user
namespaces"(SHA:4e8bbb819d1594a01f91b1de83321f68d3e6e245) I learned
that "root in a user namespace may set tc rules inside that
namespace".

I do see the CAP_NET_ADMIN check in tc_* functions has changed from
capable() to ns_capable() (which is now in term of
netlink_ns_capable())

However, in function tc_ctl_action(), the check for CAP_NET_ADMIN is
still netlink_capable which does not allow user ns root to pass this
check.

static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
             struct netlink_ext_ack *extack)
{
    ...
    if ((n->nlmsg_type != RTM_GETACTION) &&
        !netlink_capable(skb, CAP_NET_ADMIN))
        return -EPERM;
    ...
}

So is this a check missing changing for user ns?

Thanks!

Best regards,
Tianyu

On Tue, Jun 1, 2021 at 1:17 AM tianyu zhou [off-list ref] wrote:

Hi, from commit "net: Allow tc changes in user
namespaces"(SHA:4e8bbb819d1594a01f91b1de83321f68d3e6e245) I learned
that "root in a user namespace may set tc rules inside that
namespace".

I do see the CAP_NET_ADMIN check in tc_* functions has changed from
capable() to ns_capable() (which is now in term of
netlink_ns_capable())

However, in function tc_ctl_action(), the check for CAP_NET_ADMIN is
still netlink_capable which does not allow user ns root to pass this
check.

static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
             struct netlink_ext_ack *extack)
{
    ...
    if ((n->nlmsg_type != RTM_GETACTION) &&
        !netlink_capable(skb, CAP_NET_ADMIN))
        return -EPERM;
    ...
}

So is this a check missing changing for user ns?

It seems so, I do not see TC action is any different with other
TC objects here. So feel free to send a patch.

Thanks.