lkml.org 
[lkml]   [2019]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.2 342/413] fs/proc/proc_sysctl.c: fix the default values of i_uid/i_gid on /proc/sys inodes.
    Date
    From: Radoslaw Burny <rburny@google.com>

    commit 5ec27ec735ba0477d48c80561cc5e856f0c5dfaf upstream.

    Normally, the inode's i_uid/i_gid are translated relative to s_user_ns,
    but this is not a correct behavior for proc. Since sysctl permission
    check in test_perm is done against GLOBAL_ROOT_[UG]ID, it makes more
    sense to use these values in u_[ug]id of proc inodes. In other words:
    although uid/gid in the inode is not read during test_perm, the inode
    logically belongs to the root of the namespace. I have confirmed this
    with Eric Biederman at LPC and in this thread:
    https://lore.kernel.org/lkml/87k1kzjdff.fsf@xmission.com

    Consequences
    ============

    Since the i_[ug]id values of proc nodes are not used for permissions
    checks, this change usually makes no functional difference. However, it
    causes an issue in a setup where:

    * a namespace container is created without root user in container -
    hence the i_[ug]id of proc nodes are set to INVALID_[UG]ID

    * container creator tries to configure it by writing /proc/sys files,
    e.g. writing /proc/sys/kernel/shmmax to configure shared memory limit

    Kernel does not allow to open an inode for writing if its i_[ug]id are
    invalid, making it impossible to write shmmax and thus - configure the
    container.

    Using a container with no root mapping is apparently rare, but we do use
    this configuration at Google. Also, we use a generic tool to configure
    the container limits, and the inability to write any of them causes a
    failure.

    History
    =======

    The invalid uids/gids in inodes first appeared due to 81754357770e (fs:
    Update i_[ug]id_(read|write) to translate relative to s_user_ns).
    However, AFAIK, this did not immediately cause any issues. The
    inability to write to these "invalid" inodes was only caused by a later
    commit 0bd23d09b874 (vfs: Don't modify inodes with a uid or gid unknown
    to the vfs).

    Tested: Used a repro program that creates a user namespace without any
    mapping and stat'ed /proc/$PID/root/proc/sys/kernel/shmmax from outside.
    Before the change, it shows the overflow uid, with the change it's 0.
    The overflow uid indicates that the uid in the inode is not correct and
    thus it is not possible to open the file for writing.

    Link: http://lkml.kernel.org/r/20190708115130.250149-1-rburny@google.com
    Fixes: 0bd23d09b874 ("vfs: Don't modify inodes with a uid or gid unknown to the vfs")
    Signed-off-by: Radoslaw Burny <rburny@google.com>
    Acked-by: Luis Chamberlain <mcgrof@kernel.org>
    Cc: Kees Cook <keescook@chromium.org>
    Cc: "Eric W . Biederman" <ebiederm@xmission.com>
    Cc: Seth Forshee <seth.forshee@canonical.com>
    Cc: John Sperbeck <jsperbeck@google.com>
    Cc: Alexey Dobriyan <adobriyan@gmail.com>
    Cc: <stable@vger.kernel.org> [4.8+]
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/proc/proc_sysctl.c | 4 ++++
    1 file changed, 4 insertions(+)

    --- a/fs/proc/proc_sysctl.c
    +++ b/fs/proc/proc_sysctl.c
    @@ -499,6 +499,10 @@ static struct inode *proc_sys_make_inode

    if (root->set_ownership)
    root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
    + else {
    + inode->i_uid = GLOBAL_ROOT_UID;
    + inode->i_gid = GLOBAL_ROOT_GID;
    + }

    return inode;
    }

    \
     
     \ /
      Last update: 2019-07-24 22:09    [W:4.030 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site