Messages in this thread | | | Subject | Re: [PATCH v3 1/6] rtnetlink: allow RTM_SETLINK to reference other namespaces | From | Jonas Bonn <> | Date | Fri, 8 Nov 2019 09:20:40 +0100 |
| |
Hi Mahesh,
On 07/11/2019 21:36, Mahesh Bandewar (महेश बंडेवार) wrote: > On Thu, Nov 7, 2019 at 5:30 AM Jonas Bonn <jonas@norrbonn.se> wrote: >> >> >> + /* A hack to preserve kernel<->userspace interface. >> + * It was previously allowed to pass the IFLA_TARGET_NETNSID >> + * attribute as a way to _set_ the network namespace. In this >> + * case, the device interface was assumed to be in the _current_ >> + * namespace. >> + * If the device cannot be found in the target namespace then we >> + * assume that the request is to set the device in the current >> + * namespace and thus we attempt to find the device there. >> + */ > Could this bypasses the ns_capable() check? i.e. if the target is > "foo" but your current ns is bar. The process may be "capable" is foo > but the interface is not found in foo but present in bar and ends up > modifying it (especially when you are not capable in bar)?
I don't think so. There was never any capable-check for the "current" namespace so there's no change in that regard.
I do think there is an issue with this hack that I can't see any workaround for. If the user specifies an interface (by name or index) for another namespace that doesn't exist, there's a potential problem if that name/index happens to exist in the "current" namespace. In that case, one many end up inadvertently modifying the interface in the current namespace. I don't see how to avoid that while maintaining the backwards compatibility.
My absolute preference would be to drop this compat-hack altogether. iproute2 doesn't use a bare TARGET_NETNSID in this manner (for changing namespaces) and I didn't find any other users by a quick search of other prominent Netlink users: systemd, network-manager, connman. This compat-hack is there for the _potential ab-user_ of the interface, not for any known such.
> >> + if (!dev && tgt_net) { >> + net = sock_net(skb->sk); >> + if (ifm->ifi_index > 0) >> + dev = __dev_get_by_index(net, ifm->ifi_index); >> + else if (tb[IFLA_IFNAME]) >> + dev = __dev_get_by_name(net, ifname); >> + }
/Jonas
| |