Re: [PATCH] fs: Make /proc/sys inodes be owned by global root.

From: Eric W. Biederman
Date: Fri Nov 30 2018 - 09:48:35 EST


Luis Chamberlain <mcgrof@xxxxxxxxxx> writes:

> On Mon, Nov 26, 2018 at 11:29:40PM -0600, Eric W. Biederman wrote:
>> Luis Chamberlain <mcgrof@xxxxxxxxxx> writes:
>> > Thanks for the description of how to run into the issue described but
>> > is there also a practical use case today where this is happening? I ask
>> > as it would be good to know the severity of the issue in the real world
>> > today.
>>
>> People trying to run containers without a root user in the container.
>> It atypical but something doable.
>
> My question was if there are generic tools / propreitary tools which are
> doing this widely *today*. Or is this just a custom setup some folks
> use?
>
>> We spoke about this at LPC. And this is the correct behavioral change.
>>
>> The problem is there is a default value for i_uid and i_gid that is
>> correct in the general case. That default value is not corect for
>> sysctl, because proc is weird. As the sysctl permission check in
>> test_perm are all against GLOBAL_ROOT_UID and GLOBAL_ROOT_GID we did not
>> notice that i_uid and i_gid were being set wrong.
>>
>> So all this patch does is fix the default values i_uid and i_gid.
>>
>> The commit comment seems worth cleaning up. But for the
>> content of the code.
>
> The logic seems sensible then, but are we implicating what a container
> does with its sysctl values onto the entire system? If so, sure, it
> seems you want this for networking purposes as there are a series of
> sysctl values a container may want to muck with, but are we sure we
> want the same for *all* sysctl entries?

No. Please look at the patch again. It sets the default uid and gid
for sysctl entries to 0. AKA GLOBAL_ROOT_UID and GLOBAL_ROOT_GID
because there is a bug and they were not set to that value.

Those are the uids and gids that are tested agasint. It just happens
you have to be in a weird configuration for this bug to become a problem.

Eric