LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Christian Brauner <christian.brauner@ubuntu.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	avagin@virtuozzo.com, ktkhai@virtuozzo.com,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH net-next 2/2 v5] netns: restrict uevents
Date: Sun, 16 Jun 2019 09:50:57 -0700
Message-ID: <CAKdAkRRp49Qwz0CtNA6JsHdMe4Cw9tiD-ppXgqhZMBA23MzBgA@mail.gmail.com> (raw)
In-Reply-To: <875zp5rbpf.fsf@xmission.com>

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

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-29 10:44 [PATCH net-next 0/2 v5] netns: uevent filtering Christian Brauner
2018-04-29 10:44 ` [PATCH net-next 1/2 v5] uevent: add alloc_uevent_skb() helper Christian Brauner
2018-04-29 10:44 ` [PATCH net-next 2/2 v5] netns: restrict uevents Christian Brauner
2019-06-14 22:49   ` Dmitry Torokhov
2019-06-16 11:50     ` ebiederm
2019-06-16 16:50       ` Christian Brauner
2019-06-16 17:14         ` Dmitry Torokhov
2019-06-16 16:50       ` Dmitry Torokhov [this message]
2018-04-30 15:55 ` [PATCH net-next 0/2 v5] netns: uevent filtering ebiederm
2018-05-01 14:23   ` David Miller

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKdAkRRp49Qwz0CtNA6JsHdMe4Cw9tiD-ppXgqhZMBA23MzBgA@mail.gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=avagin@virtuozzo.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=serge@hallyn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox