linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: dhowells@redhat.com, Andreas Dilger <adilger@sun.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	akpm@linux-foundation.org, linux-afs@lists.infradead.org,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s
Date: Wed, 17 Jun 2009 19:03:43 +0100	[thread overview]
Message-ID: <28259.1245261823@redhat.com> (raw)
In-Reply-To: <alpine.LFD.2.01.0906171027470.16802@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> What _I_ mean is that THIS IS IMPOSSIBLE TO DO FROM USER SPACE!
> 
> Try it. Not doable. User space simply doesn't know enough, and has 
> fundamental races with mount/umount.

Ummm...  I'm not sure I completely agree.  If you've managed to open, say,
"/afs", where's the race with mount/umount?  You've got a file descriptor you
can use as a handle.  Yes, you have to check that it's actually an inode of
your fs, but that's not exactly difficult, and that's not going to change just
because someone unmounts it or mounts over it whilst you've got it open.

However, that makes userspace have to assume that the pioctl handler is on an
AFS inode, perhaps any AFS inode.  This is not compatible with OpenAFS as it
stands, and also means you can't use the AFS pioctls before mounting anything,
and you can't mount it elsewhere and expect it to work.

> End result: you do need a new system call. 
> 
> I just don't think "pioctl()" is a good one.

Out of interest, why not?  Is it just because it's another multipexor?  Or is
it because it's been abused to have pathless commands?

> You'd be better off with some modification of open and then use ioctl.

So you'd say use:

	fd = open("/the/target/file", O_SUPPRESS | (nofollow?O_NOFOLLOW:0));
	ioctl(fd, cmd, &args);
	close(fd);

where O_SUPPRESS (or whatever) suppressed override of the ops tables by the
chardev and blockdev handlers, and allows symlinks to be opened, rather than:

	pioctl("/the/target/file", cmd, &args, nofollow);

I would counter that with:

 (1) pioctl() is actually simpler and cleaner, and doesn't require
     modifications to open().

 (2) The open()/ioctl() method doesn't handle pathless pioctls, and so is not
     a complete solution.

 (3) The open()/ioctl() method assumes that pioctl() command numbers don't
     clash with ioctl() command numbers - something that's unfortunately not
     true of OpenAFS:-(

     Of course, you could have one ioctl() command number that says that this
     is a pioctl() and then a second number in the argument data that is the
     pioctl() command number.

 (4) pioctl() is compatible with OpenAFS.

Do you also disagree with OpenAFS's idea of creating a proc file to open so
that you can do ioctls on that to emulate pioctl()?  That would serve also.

David

  parent reply	other threads:[~2009-06-17 18:06 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-16 20:38 [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s David Howells
2009-06-16 20:38 ` [PATCH 01/17] VFS: Implement the pioctl() system call David Howells
2009-06-16 20:54   ` Christoph Hellwig
2009-06-17  0:19   ` David Howells
2009-06-17  9:02     ` Alan Cox
2009-06-16 20:38 ` [PATCH 02/17] VFS: Implement the AFS " David Howells
2009-06-16 20:39 ` [PATCH 03/17] VFS: Implement handling for pathless pioctls David Howells
2009-06-17  7:47   ` Andreas Dilger
2009-06-17 18:26   ` David Howells
2009-06-16 20:39 ` [PATCH 04/17] AFS: Add key request for pioctl David Howells
2009-06-16 20:39 ` [PATCH 05/17] AFS: Handle pathless pioctls aimed at AFS David Howells
2009-06-16 20:39 ` [PATCH 06/17] VFS: Define pioctl command wrappers David Howells
2009-06-16 20:39 ` [PATCH 07/17] AFS: Implement the PGetFid pioctl David Howells
2009-06-16 20:39 ` [PATCH 08/17] AFS: Implement the PGetFileCell pioctl David Howells
2009-06-16 20:39 ` [PATCH 09/17] AFS: Implement the PGetVolStat pioctl David Howells
2009-06-16 20:39 ` [PATCH 10/17] AFS: Implement the PWhereIs pioctl David Howells
2009-06-17  7:51   ` Andreas Dilger
2009-06-17 18:05   ` David Howells
2009-06-16 20:39 ` [PATCH 11/17] AFS: Implement the PFlushCB pioctl David Howells
2009-06-16 20:39 ` [PATCH 12/17] KEYS: Export lookup_user_key() and the key permission request flags David Howells
2009-06-16 20:39 ` [PATCH 13/17] RxRPC: Record extra data in key David Howells
2009-06-16 20:39 ` [PATCH 14/17] RxRPC: Declare the security index constants symbolically David Howells
2009-06-16 20:40 ` [PATCH 15/17] AFS: Implement the PSetTokens pioctl David Howells
2009-06-16 20:40 ` [PATCH 16/17] KEYS: Add a function by which the contents of a keyring can be enumerated David Howells
2009-06-16 20:40 ` [PATCH 17/17] AFS: Implement the PGetTokens pioctl David Howells
2009-06-16 22:59 ` [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s David Howells
2009-06-16 23:11   ` Alan Cox
2009-06-17  0:25   ` David Howells
2009-06-17  7:55     ` Andreas Dilger
2009-06-17 16:09       ` Linus Torvalds
2009-06-17 18:37         ` Al Viro
2009-06-17 18:44           ` Linus Torvalds
2009-06-17 18:52             ` Al Viro
2009-06-17 19:28             ` David Howells
2009-06-18 12:50               ` Olivier Galibert
2009-06-17 17:24       ` David Howells
2009-06-17 17:33         ` Linus Torvalds
2009-06-17 18:03         ` David Howells [this message]
2009-06-17 18:24           ` Linus Torvalds
2009-06-17 18:30           ` Theodore Tso
2009-06-17 19:14             ` david
2009-06-17 19:30             ` David Howells
2009-06-17 19:51           ` David Howells
2009-06-17 20:09             ` Linus Torvalds
2009-06-17  9:00     ` Alan Cox

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=28259.1245261823@redhat.com \
    --to=dhowells@redhat.com \
    --cc=adilger@sun.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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).