[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aNPppN7crz-n7bej@shredder>
Date: Wed, 24 Sep 2025 15:52:52 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Johannes Wiesböck <johannes.wiesboeck@...ec.fraunhofer.de>
Cc: gyroidos@...ec.fraunhofer.de, sw@...onwunderlich.de,
Michael Weiß <michael.weiss@...ec.fraunhofer.de>,
Harshal Gohel <hg@...onwunderlich.de>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>,
Kuniyuki Iwashima <kuniyu@...gle.com>,
Stanislav Fomichev <sdf@...ichev.me>,
Xiao Liang <shaw.leon@...il.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rtnetlink: Allow deleting FDB entries in user namespace
On Tue, Sep 23, 2025 at 10:21:40AM +0200, Johannes Wiesböck wrote:
> Deletion of FDB entries requires CAP_NET_ADMIN, yet, processes in a
> non-initial user namespace receive an EPERM because the capability is
> always checked against the initial user namespace. This restricts the
> FDB management from unprivileged containers.
It's worth mentioning that unprivileged containers can add FDB entries,
but not delete them:
$ id
uid=1000(idosch) gid=1000(idosch) groups=1000(idosch),10(wheel)
$ unshare -Urn
$ id
uid=0(root) gid=0(root) groups=0(root),65534(nobody)
$ ip link add name br0 up type bridge
$ bridge fdb add 00:11:22:33:44:55 dev br0 self permanent
$ bridge fdb del 00:11:22:33:44:55 dev br0 self permanent
RTNETLINK answers: Operation not permitted
After (not exactly your patch, see below):
$ id
uid=1000(idosch) gid=1000(idosch) groups=1000(idosch),10(wheel)
$ unshare -Urn
$ id
uid=0(root) gid=0(root) groups=0(root),65534(nobody)
$ ip link add name br0 up type bridge
$ bridge fdb add 00:11:22:33:44:55 dev br0 self permanent
$ bridge fdb del 00:11:22:33:44:55 dev br0 self permanent
$ echo $?
0
>
> Replace netlink_capable with netlink_net_capable that performs the
> capability check on the user namespace the netlink socket was opened in.
>
> This patch was tested using a container on GyroidOS, where it was
> possible to delete FDB entries from an unprivileged user namespace and
> private network namespace.
>
> Reviewed-by: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
> Tested-by: Harshal Gohel <hg@...onwunderlich.de>
> Signed-off-by: Johannes Wiesböck <johannes.wiesboeck@...ec.fraunhofer.de>
> ---
> net/core/rtnetlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 094b085cff206..2f96258bd4fd7 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4707,7 +4707,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
> int err;
> u16 vid;
>
> - if (!netlink_capable(skb, CAP_NET_ADMIN))
> + if (!netlink_net_capable(skb, CAP_NET_ADMIN))
> return -EPERM;
AFAICT, before commit 1690be63a27b ("bridge: Add vlan support to static
neighbors") it was possible for unprivileged containers to delete FDB
entries and the commit message does not say anything about why this
check was added. So, unless I'm missing something, I think your patch
should be treated as a fix and targeted at "net". See:
https://docs.kernel.org/process/maintainer-netdev.html
It might be a rebase issue. Commit c5c351088ae7 ("netns: fdb: allow
unprivileged users to add/del fdb entries") removed the CAP_NET_ADMIN
check from both rtnl_fdb_add() and rtnl_fdb_del() about two weeks before
v11 of 1690be63a27b was applied.
Also, there's already a netlink_net_capable(skb, CAP_NET_ADMIN) check in
rtnetlink_rcv_msg(), so I would just remove the capability check from
rtnl_fdb_del().
Powered by blists - more mailing lists