On Tue, Jan 7, 2025 at 4:57 PM Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote: > > From: Xiao Liang <shaw.leon@xxxxxxxxx> > Date: Sat, 4 Jan 2025 20:57:21 +0800 [...] > > - In amt_newlink() drivers/net/amt.c: > > > > amt->net = net; > > ... > > amt->stream_dev = dev_get_by_index(net, ... > > > > Uses net, but amt_lookup_upper_dev() only searches in dev_net. > > So the AMT device may not be properly deleted if it's in a different > > netns from lower dev. > > I think you are right, and the upper device will be leaked > and UAF will happen. > > amt must manage a list linked to a lower dev. > > Given no one has reported the issue, another option would be > drop cross netns support in a short period. Yes. I also noticed AMT sets dev->netns_local to prevent netns change. Probably it also assumes the same netns during creation. [...] > > > > - In gtp_newlink() in drivers/net/gtp.c: > > > > gtp->net = src_net; > > ... > > gn = net_generic(dev_net(dev), gtp_net_id); > > list_add_rcu(>p->list, &gn->gtp_dev_list); > > > > Uses src_net, but priv is linked to list in dev_net. So it may not be > > properly deleted on removal of link netns. > > The device is linked to a list in the same netns, so the > device will not be leaked. See gtp_net_exit_batch_rtnl(). > > Rather, the problem is the udp tunnel socket netns could be > freed earlier than the dev netns. Yes, you're right. Actually I mean the netns of the socket by "link netns" (there's some clarification about this in patch 02). [...] > > > > - In pfcp_newlink() in drivers/net/pfcp.c: > > > > pfcp->net = net; > > ... > > pn = net_generic(dev_net(dev), pfcp_net_id); > > list_add_rcu(&pfcp->list, &pn->pfcp_dev_list); > > > > Same as above. > > I haven't tested pfcp but it seems to have the same problem. > > I'll post patches for gtp and pfcp. > It would be nice. > > > > > - In lowpan_newlink() in net/ieee802154/6lowpan/core.c: > > > > wdev = dev_get_by_index(dev_net(ldev), nla_get_u32(tb[IFLA_LINK])); > > > > Looks for IFLA_LINK in dev_net, but in theory the ifindex is defined > > in link netns. > > I guess you mean the ifindex is defined in src_net instead. > Not sure if it's too late to change the behaviour. Yes, it's source net for lowpan. I think it depends on whether the interpretation of IFLA_LINK should be considered as part API provided by rtnetlink core, or something customizable by driver. In the former case, this can be considered as a bug. Thanks.
