lkml.org 
[lkml]   [2019]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH net-next 2/2 v5] netns: restrict uevents
    Hi Eric,

    On Sun, Jun 16, 2019 at 4:50 AM Eric W. Biederman <ebiederm@xmission.com> wrote:
    >
    > Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
    >
    > > Hi Christian,
    > >
    > > On Sun, Apr 29, 2018 at 3:45 AM Christian Brauner
    > > <christian.brauner@ubuntu.com> wrote:
    > >>
    > >> commit 07e98962fa77 ("kobject: Send hotplug events in all network namespaces")
    > >>abhishekbh@google.com
    > >> 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. 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.
    > >> Currently, only network devices carry namespace tags (i.e. network
    > >> namespace tags). Hence, uevents for network devices only show up in the
    > >> network namespace such devices are created in or moved to.
    > >>
    > >> 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 simplifies and fixes couple of things:
    > >> - Split codepath for sending uevents by kobject namespace tags:
    > >> 1. Untagged kobjects - uevent_net_broadcast_untagged():
    > >> Untagged kobjects will be broadcast into all uevent sockets recorded
    > >> in uevent_sock_list, i.e. into all network namespacs owned by the
    > >> intial user namespace.
    > >> 2. Tagged kobjects - uevent_net_broadcast_tagged():
    > >> Tagged kobjects will only be broadcast into the network namespace they
    > >> were tagged with.
    > >> Handling of tagged kobjects in 2. does not cause any semantic changes.
    > >> This is just splitting out the filtering logic that was handled by
    > >> kobj_bcast_filter() before.
    > >> Handling of untagged kobjects in 1. will cause a semantic change. The
    > >> reasons why this is needed and ok have been discussed in [1]. Here is a
    > >> short summary:
    > >> - Userspace ignores uevents from network namespaces that are not owned by
    > >> the intial user namespace:
    > >> 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.
    > >
    > > Unfortunately udev is not the only consumer of uevents, for example on
    > > Android there is healthd that also consumes uevents, and this
    > > particular change broke Android running in a container on Chrome OS.
    > > Can this be reverted? Or, if we want to keep this, how can containers
    > > that use separate user namespace still listen to uevents?
    >
    > The code has been in the main tree for over a year so at a minimum
    > reverting this has the real chance of causing a regression for
    > folks like lxc.
    >
    > I don't think Android running in a container on Chrome OS was even
    > available when this change was merged. So I don't think this falls
    > under the ordinary no regression rules.
    >
    > I may be wrong but I think this is a case of developing new code on an
    > old kernel and developing a dependence on a bug that had already been
    > fixed in newer kernels.

    No, this is not quite the case. We have been shipping Android on
    Chrome OS since 2016, the concept of running Android in a container
    definitely predates these series of patches.

    > I know Christian did his best to reach out to
    > everyone when this change came through, so only getting a bug report
    > over a year after the code was merged is concerning.

    This only proves that it is hard to change userspace-visible behavior
    as one can't really know who might be using the interfaces and for
    what reason. Again, udev is not the only consumer of uevents; as fat
    as I know Android does not use udev and there are other users of
    uevents as well. For example, libusb can be compiled to listen to
    uevents directly.

    >
    > That said uevents should be completely useless in a user namespace
    > except as letting you know something happened. Is that what healthd
    > is using them for?

    Yes, that is one of the use cases. Appearance of AC power supply can
    be used to adjust system behavior, for example.

    Thanks.

    --
    Dmitry

    \
     
     \ /
      Last update: 2019-06-16 18:52    [W:2.688 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site