linux-kernel - Re: Regression Mounting /proc w/ kernel 3.4.0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 25 May 2012 14:42:59 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	kd6lvw@...oo.com, linux-kernel@...r.kernel.org,
	Vasiliy Kulikov <segoon@...nwall.com>,
	Arkadiusz Miskiewicz <a.miskiewicz@...il.com>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: Regression Mounting /proc w/ kernel 3.4.0 - "uid" parameter no
 longer ignored.

Heh.

If it is *only* uid, maybe we should just expressly ignore it. Like
the (completely untested) attached patch.

Hmm?

                  Linus

On Fri, May 25, 2012 at 1:19 PM, Eric W. Biederman
<ebiederm@...ssion.com> wrote:
> kd6lvw@...oo.com writes:
>
>> --- On Thu, 5/24/12, Eric W. Biederman <ebiederm@...ssion.com> wrote:
>>> "D. Stussy" <spam+newsgroups@...-arc.ampr.org> writes:
>>> > proc:  unrecognized mount option "uid=1" or missing value
>>> >
>>> > This works under 3.3.6 and earlier (I never tried 3.3.7).  It appears that the
>>> > "uid" mount option was removed.  WHY?  More importantly, where is this change
>>> > listed?  I didn't see it in the git log.
>>> >
>>> > This is really bad as I rebooted a remotely colocated server and since the
>>> > procfs is not mounted, I cannot log in via SSH to correct the problem.
>>> >
>>> > OK, so there's not that much point in changing the UID of the procfs files, but
>>> > I preferred to set their default ownership to a non-privileged user.
>>>
>>> There has never been a uid= option to /proc in Linus's tree.  I believe
>>> if you look you will find this feature was from a patch (perhaps from
>>> your distro) that added that support.
>>
>> I compile both the util-linux-ng and the kernel from source.  There is no distribution patch involved.  As noted, kernel version 3.3.6 didn't bitch about the setting, while 3.4.0 does and won't mount procfs.
>>
>> From the man page (copy at "http://linux.die.net/man/8/mount"):
>>
>> "Mount options for proc
>>
>>      "uid=value and gid=value
>>
>> "These options are recognized, but have no effect as far as I can see."
>>
>>
>> Removal of this compatibility should have been noted.  WHERE was it
>> noted?  Linux.die.net is a distribution independent documentation
>> site.  It might not do anything, but per mount's documentation, it is
>> acceptable.
>
> The only possible change to /proc that could have cause this behavior
> between 3.3 and 3.4 is below.
>
> Not ignoring options seems to be a regression that has affected your
> setup on 3.4.
>
> My gut feel is that we should revert all of the /proc option parsing
> code as it is ugly code and apparently it could not have been used
> before 3.4 so no one can be depending on it.  But I just want an excuse
> to get rid of ugly code.  Accepting that /proc ignores all options
> passed ot it as part of the ABI and not being able to add any options
> to /proc ever is also a pretty horrible state to be in.
>
> Shrug I will let you and the parties involved in creating this
> regression figure this out.
>
> Eric
>
> ommit 99663be772c827b8f5f594fe87eb4807be1994e5
> Author: Vasiliy Kulikov <segoon@...nwall.com>
> Date:   Thu Apr 5 14:25:04 2012 -0700
>
>    proc: fix mount -t proc -o AAA
>
>    The proc_parse_options() call from proc_mount() runs only once at boot
>    time.  So on any later mount attempt, any mount options are ignored
>    because ->s_root is already initialized.
>
>    As a consequence, "mount -o <options>" will ignore the options.  The
>    only way to change mount options is "mount -o remount,<options>".
>
>    To fix this, parse the mount options unconditionally.
>
>    Signed-off-by: Vasiliy Kulikov <segoon@...nwall.com>
>    Reported-by: Arkadiusz Miskiewicz <a.miskiewicz@...il.com>
>    Tested-by: Arkadiusz Miskiewicz <a.miskiewicz@...il.com>
>    Cc: Alexey Dobriyan <adobriyan@...il.com>
>    Cc: Al Viro <viro@...iv.linux.org.uk>
>    Cc: Valdis Kletnieks <Valdis.Kletnieks@...edu>
>    Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
>    Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
>
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 46a15d8..eed44bf 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -115,12 +115,13 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
>        if (IS_ERR(sb))
>                return ERR_CAST(sb);
>
> +       if (!proc_parse_options(options, ns)) {
> +               deactivate_locked_super(sb);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
>        if (!sb->s_root) {
>

Download attachment "patch.diff" of type "application/octet-stream" (882 bytes)