linux-kernel - Re: [PATCH -V18 04/13] vfs: Allow handle based open on symlinks

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] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 21 Aug 2010 10:09:00 +1000
From:	Neil Brown <neilb@...e.de>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Christoph Hellwig <hch@...radead.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	adilger@....com, corbet@....net, npiggin@...nel.dk,
	hooanon05@...oo.co.jp, bfields@...ldses.org, miklos@...redi.hu,
	linux-fsdevel@...r.kernel.org, sfrench@...ibm.com,
	philippe.deniel@....FR, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -V18 04/13] vfs: Allow handle based open on symlinks

[[email address for Nick Piggin changed to npiggin@...nel.dk]]

On Fri, 20 Aug 2010 12:51:35 +0100
Al Viro <viro@...IV.linux.org.uk> wrote:

> On Fri, Aug 20, 2010 at 07:53:03PM +1000, Neil Brown wrote:
> > On Fri, 20 Aug 2010 04:30:57 -0400
> > Christoph Hellwig <hch@...radead.org> wrote:
> > 
> > > Suddenly getting an file pointer for a symlink which could never happen
> > > before is a really bad idea.  Just add a proper readlink_by_handle
> > > system call, similar to what's done in the XFS interface.
> > 
> > Why is that?
> > With futexes we suddenly get a file descriptor for something we could never
> > get a file descriptor on before and that doesn't seem to be a problem.
> > 
> > Why should symlinks be special as the only thing that you cannot have a file
> > descriptor for?  Uniformity of interface is a very valuable property.
> 
> You are welcome to review the codepaths around pathname resolution for
> assumptions of presense of ->follow_link() and friends; there _are_
> subtle cases and dumping your "opened symlinks" in there is far from
> a trivial change.  Note that it affects more than just the starting
> points of lookups; /proc/*/fd/* stuff is also involved.

So as I understand it you aren't rejecting the concept in principle, but you
believe non-trivial code review is required before it can be considered an
acceptable change?
That's quite reasonable.  I hope to find time to have a look at the code.

> 
> BTW, speaking of NULL pathname, linkat() variant that allows creating a link
> to an opened file is also a very dubious thing; at the very least, you get
> non-trivial security implications, since now a process that got an opened
> descriptor passed to it by somebody else may create hardlinks to the sucker.

Fair comment, and while one could imagine ways around this (such as requiring
some Capability to link an fd) they wouldn't be very elegant.
But then nor is inventing a pile of new syscalls for doing different things
with handles such as the list Aneesh posted.

Maybe a different approach is needed.

How about a new AT flag:  AT_FILE_HANDLE

Meaning is that the 'dirfd' is used only to identify a filesystem (vfsmnt) and
the 'name' pointer actually points to a filehandle fragment interpreted in
that filesystem.

One problem is that there is no way to pass the length...
Options:
   fragment is at most 64 bytes nul padded at the end
   fragment is hex encoded and nul terminated
   ??

I think I prefer the hex encoding, but I'm hoping someone else has a better
idea.

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