linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Neil Brown <neilb@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	adilger@sun.com, corbet@lwn.net, npiggin@kernel.dk,
	hooanon05@yahoo.co.jp, bfields@fieldses.org, miklos@szeredi.hu,
	linux-fsdevel@vger.kernel.org, sfrench@us.ibm.com,
	philippe.deniel@CEA.FR, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -V18 04/13] vfs: Allow handle based open on symlinks
Date: Sat, 21 Aug 2010 10:09:00 +1000	[thread overview]
Message-ID: <20100821100900.4b15fe08@notabene> (raw)
In-Reply-To: <20100820115135.GQ31363@ZenIV.linux.org.uk>

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

On Fri, 20 Aug 2010 12:51:35 +0100
Al Viro <viro@ZenIV.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@infradead.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

  reply	other threads:[~2010-08-21  0:09 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-20  1:51 [PATCH -V18 0/13] Generic name to handle and open by handle syscalls Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 01/13] exportfs: Return the minimum required handle size Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 02/13] vfs: Add name to file handle conversion support Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 03/13] vfs: Add open by file handle support Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 04/13] vfs: Allow handle based open on symlinks Aneesh Kumar K.V
2010-08-20  2:13   ` Aneesh Kumar K. V
2010-08-20  6:53     ` Aneesh Kumar K. V
2010-08-20  8:30   ` Christoph Hellwig
2010-08-20  9:53     ` Neil Brown
2010-08-20 11:51       ` Al Viro
2010-08-21  0:09         ` Neil Brown [this message]
2010-08-21  7:13           ` Andreas Dilger
2010-08-21  9:32             ` Aneesh Kumar K. V
2010-08-22 23:06             ` Neil Brown
2010-08-23  1:24               ` Aneesh Kumar K. V
2010-08-23  1:52                 ` Neil Brown
2010-08-24 10:40                   ` Aneesh Kumar K. V
2010-08-23  2:49               ` Aneesh Kumar K. V
2010-08-25  2:06                 ` Neil Brown
2010-08-24  9:41               ` Bastien ROUCARIES
2010-08-25  2:04                 ` Neil Brown
2010-08-25  9:13                   ` Bastien ROUCARIES
2010-08-21  8:30           ` Nick Piggin
2010-08-21  9:42             ` Aneesh Kumar K. V
2010-08-22  2:02               ` Aneesh Kumar K. V
2010-08-24  7:21               ` Nick Piggin
2010-08-24 10:34                 ` Aneesh Kumar K. V
2010-08-24 13:19                 ` J. Bruce Fields
2010-08-22 23:17             ` Neil Brown
2010-08-24  7:29               ` Nick Piggin
2010-08-21  9:31           ` Aneesh Kumar K. V
2010-08-20 13:25       ` Peter Zijlstra
2010-08-20 23:47         ` Neil Brown
2010-08-20 14:38     ` Aneesh Kumar K. V
2010-08-20  1:51 ` [PATCH -V18 05/13] vfs: Support null pathname in readlink Aneesh Kumar K.V
2010-08-20  8:32   ` Christoph Hellwig
2010-08-20 10:04     ` Neil Brown
2010-08-20 14:43     ` Aneesh Kumar K. V
2010-08-20  1:51 ` [PATCH -V18 06/13] vfs: Support null pathname in faccessat Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 07/13] vfs: Support null pathname in linkat Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 08/13] x86: Add new syscalls for x86_32 Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 09/13] x86: Add new syscalls for x86_64 Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 10/13] unistd.h: Add new syscalls numbers to asm-generic Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 11/13] vfs: Export file system uuid via /proc/<pid>/mountinfo Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 12/13] ext3: Copy fs UUID to superblock Aneesh Kumar K.V
2010-08-20  1:51 ` [PATCH -V18 13/13] ext4: " Aneesh Kumar K.V

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100821100900.4b15fe08@notabene \
    --to=neilb@suse.de \
    --cc=adilger@sun.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=bfields@fieldses.org \
    --cc=corbet@lwn.net \
    --cc=hch@infradead.org \
    --cc=hooanon05@yahoo.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=npiggin@kernel.dk \
    --cc=philippe.deniel@CEA.FR \
    --cc=sfrench@us.ibm.com \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).