LKML Archive on lore.kernel.org
 help / color / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dmitry Torokhov <dmitry.torokhov@gmail.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 06:50:20 -0500
Message-ID: <875zp5rbpf.fsf@xmission.com> (raw)
In-Reply-To: <CAKdAkRTtffEQfZLnSW9CwzX_oYzHdOE816OvciGadqV7RHaV1Q@mail.gmail.com>

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.  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.

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?


One solution would be to tweak the container userspace on ChromeOS to
listen to the uevents outside the container and to relay them into the
Android container.

Eric

  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 [this message]
2019-06-16 16:50       ` Christian Brauner
2019-06-16 17:14         ` Dmitry Torokhov
2019-06-16 16:50       ` Dmitry Torokhov
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=875zp5rbpf.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=avagin@virtuozzo.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=davem@davemloft.net \
    --cc=dmitry.torokhov@gmail.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