On Wed, Apr 25, 2018, 00:41 Eric W. Biederman wrote: > > Bah. This code is obviously correct and probably wrong. > > How do we deliver uevents for network devices that are outside of the > initial user namespace? The kernel still needs to deliver those. > > The logic to figure out which network namespace a device needs to be > delivered to is is present in kobj_bcast_filter. That logic will almost > certainly need to be turned inside out. Sign not as easy as I would > have hoped. My first patch that we discussed put additional filtering logic into kobj_bcast_filter for that very reason. But I can move that logic out and come up with a new patch. > Eric > > Christian Brauner writes: > > commit 07e98962fa77 ("kobject: Send hotplug events in all network > namespaces") > > > > enabled sending hotplug events into all network namespaces back in 2010. > > Over time the set of uevents that get sent into all network namespaces > has > > shrunk a little. We have now reached the point where hotplug events for > all > > devices that carry a namespace tag are filtered according to that > > namespace. Specifically, they are filtered whenever the namespace tag of > > the kobject does not match the namespace tag of the netlink socket. One > > example are network devices. Uevents for network devices only show up in > > the network namespaces these devices are moved to or created in. > > > > However, any uevent for a kobject that does not have a namespace tag > > associated with it will not be filtered and we will broadcast it into all > > network namespaces. This behavior stopped making sense when user > namespaces > > were introduced. > > > > This patch restricts uevents to the initial user namespace for a couple > of > > reasons that have been extensively discusses on the mailing list [1]. > > - Thundering herd: > > Broadcasting uevents into all network namespaces introduces significant > > overhead. > > All processes that listen to uevents running in non-initial user > > namespaces will end up responding to uevents that will be meaningless > to > > them. Mainly, because non-initial user namespaces cannot easily manage > > devices unless they have a privileged host-process helping them out. > This > > means that there will be a thundering herd of activity when there > > shouldn't be any. > > - Uevents from non-root users are already filtered in userspace: > > Uevents are filtered by userspace in a user namespace because the > > received uid != 0. Instead the uid associated with the event will be > > 65534 == "nobody" because the global root uid is not mapped. > > This means we can safely and without introducing regressions modify the > > kernel to not send uevents into all network namespaces whose owning > user > > namespace is not the initial user namespace because we know that > > userspace will ignore the message because of the uid anyway. I have > > a) verified that is is true for every udev implementation out there b) > > that this behavior has been present in all udev implementations from > the > > very beginning. > > - Removing needless overhead/Increasing performance: > > Currently, the uevent socket for each network namespace is added to the > > global variable uevent_sock_list. The list itself needs to be protected > > by a mutex. So everytime a uevent is generated the mutex is taken on > the > > list. The mutex is held *from the creation of the uevent (memory > > allocation, string creation etc. until all uevent sockets have been > > handled*. This is aggravated by the fact that for each uevent socket > that > > has listeners the mc_list must be walked as well which means we're > > talking O(n^2) here. Given that a standard Linux workload usually has > > quite a lot of network namespaces and - in the face of containers - a > lot > > of user namespaces this quickly becomes a performance problem (see > > "Thundering herd" above). By just recording uevent sockets of network > > namespaces that are owned by the initial user namespace we > significantly > > increase performance in this codepath. > > - Injecting uevents: > > There's a valid argument that containers might be interested in > receiving > > device events especially if they are delegated to them by a privileged > > userspace process. One prime example are SR-IOV enabled devices that > are > > explicitly designed to be handed of to other users such as VMs or > > containers. > > This use-case can now be correctly handled since > > commit 692ec06d7c92 ("netns: send uevent messages"). This commit > > introduced the ability to send uevents from userspace. As such we can > let > > a sufficiently privileged (CAP_SYS_ADMIN in the owning user namespace > of > > the network namespace of the netlink socket) userspace process make a > > decision what uevents should be sent. This removes the need to blindly > > broadcast uevents into all user namespaces and provides a performant > and > > safe solution to this problem. > > - Filtering logic: > > This patch filters by *owning user namespace of the network namespace a > > given task resides in* and not by user namespace of the task per se. > This > > means if the user namespace of a given task is unshared but the network > > namespace is kept and is owned by the initial user namespace a listener > > that is opening the uevent socket in that network namespace can still > > listen to uevents. > > > > [1]: https://lkml.org/lkml/2018/4/4/739 > > Signed-off-by: Christian Brauner > > --- > > Changelog v1->v2: > > * patch unchanged > > Changelog v0->v1: > > * patch unchanged > > --- > > lib/kobject_uevent.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c > > index 15ea216a67ce..f5f5038787ac 100644 > > --- a/lib/kobject_uevent.c > > +++ b/lib/kobject_uevent.c > > @@ -703,9 +703,13 @@ static int uevent_net_init(struct net *net) > > > > net->uevent_sock = ue_sk; > > > > - mutex_lock(&uevent_sock_mutex); > > - list_add_tail(&ue_sk->list, &uevent_sock_list); > > - mutex_unlock(&uevent_sock_mutex); > > + /* Restrict uevents to initial user namespace. */ > > + if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) { > > + mutex_lock(&uevent_sock_mutex); > > + list_add_tail(&ue_sk->list, &uevent_sock_list); > > + mutex_unlock(&uevent_sock_mutex); > > + } > > + > > return 0; > > } > > > > @@ -713,9 +717,11 @@ static void uevent_net_exit(struct net *net) > > { > > struct uevent_sock *ue_sk = net->uevent_sock; > > > > - mutex_lock(&uevent_sock_mutex); > > - list_del(&ue_sk->list); > > - mutex_unlock(&uevent_sock_mutex); > > + if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) { > > + mutex_lock(&uevent_sock_mutex); > > + list_del(&ue_sk->list); > > + mutex_unlock(&uevent_sock_mutex); > > + } > > > > netlink_kernel_release(ue_sk->sk); > > kfree(ue_sk); >