linux-kernel - 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87wr40ko8j.fsf@xmission.com>
Date:	Fri, 25 May 2012 14:19:24 -0600
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	kd6lvw@...oo.com
Cc:	Vasiliy Kulikov <segoon@...nwall.com>,
	Arkadiusz Miskiewicz <a.miskiewicz@...il.com>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Regression Mounting /proc w/ kernel 3.4.0 - "uid" parameter no longer ignored.

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) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/