linux-kernel - Re: [PATCH 0/3] vfs: plug some holes involving LAST_BIND symlinks and file bind mounts (try #5)

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: <m1ljhwbyr3.fsf@fess.ebiederm.org>
Date:	Mon, 23 Nov 2009 15:35:44 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Jeff Layton <jlayton@...hat.com>
Cc:	Jamie Lokier <jamie@...reable.org>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, pavel@....cz, miklos@...redi.hu,
	viro@...IV.linux.org.uk
Subject: Re: [PATCH 0/3] vfs: plug some holes involving LAST_BIND symlinks and file bind mounts (try #5)

Jeff Layton <jlayton@...hat.com> writes:

> On Mon, 23 Nov 2009 22:49:48 +0000
> Jamie Lokier <jamie@...reable.org> wrote:
>
>> Jeff Layton wrote:
>> > > check_path_accessible seems pretty horrible.   If a process is running
>> > > inside of a subdirectory it doesn't have permissions to access, say
>> > > a chroot, /proc/self/fd/XXX becomes completely unusable.
>> > > 
>> > 
>> > Hmm...I have this in there:
>> > 
>> > +		/* are we at global root or root of namespace? */
>> > +		if ((tdentry == root.dentry && vfsmnt == root.mnt) ||
>> > +		    vfsmnt->mnt_parent == vfsmnt)
>> > +			break;
>> > 
>> > ...In the case of a chroot, wouldn't "current->fs->root" point to the
>> > root of the process' namespace? Or am I misunderstanding what
>> > current->fs actually represents?
>> 
>> A process can run inside a subdirectory it doesn't have permissions to
>> access without that being a chroot.
>> 
>
> Certainly.
>
>> It can also run inside a subdirectory that isn't accessible from it's
>> root, if that's how it was started - as well as having other
>> descriptors pointing to things outside its root.
>> 
>
> Yes.
>
>> It can also be passed file descriptors from outside it's root while
>> it's running.
>> 
>
> Yep.
>
>> Really, I think the /proc/PID/fd/N check should restrict the open to
>> the O_* limitations that were used to open fd N before, and not have
>> any connection to actual paths at the time of this check.
>> 
>
> The big question with all of this is: Should a task have the ability
> to follow a /proc/pid symlink to a path that it wouldn't ordinarily be
> able to resolve with a path lookup. The concensus that I got from the
> bugtraq discussion was that it should not, and this patch is an attempt
> to prevent that.
>
> I take it from you and Eric's comments that you disagree? If so, what's
> your rationale for allowing a task to resolve this symlink when it
> wouldn't ordinarily be able to do so if it were a "normal" symlink?

For myself I start with the simple fact that the code has worked the
way it currently does since around linux 0.99.  We have to be very
careful if we change this to avoid breaking existing applications.

So if we change the existing behaviour it must be done in such a way
that legitimate applications do not break.

Eric
--
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/