Implement a new open flag that doesn't require any access on the file.
It works on any file type including symlinks.
The sole purpose is to help race free "userspace lookup" type
operations. So fstat, fch*, *at work but nothing else. Filesystem's
->open() is not called and f_op is set to NULL.
It would be logical to reuse the open_flag=3 value, but that has
historically been used with different semantics so I'm afraid of
touching it.
Not sure if it's a good base for what AFS/pioctl is trying to do, but
it's something that I personally would very much like to see.
Comments?
Thanks,
Miklos
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c 2009-06-12 09:30:49.000000000 +0200
+++ linux-2.6/fs/namei.c 2009-06-18 08:16:55.000000000 +0200
@@ -1651,6 +1651,38 @@ static int open_will_write_to_fs(int fla
return (flag & O_TRUNC);
}
+static struct file *open_noaccess(int dfd, const char *name, int flags)
+{
+ int err;
+ struct nameidata nd;
+ struct file *filp = get_empty_filp();
+ struct inode *inode;
+
+ err = -ENFILE;
+ if (filp == NULL)
+ goto out_err;
+
+ err = do_path_lookup(dfd, name, lookup_flags(flags), &nd);
+ if (err)
+ goto out_put_filp;
+
+ inode = nd.path.dentry->d_inode;
+ filp->f_flags = flags & (O_NOACC | O_NOFOLLOW | O_DIRECTORY);
+ filp->f_mode = 0;
+ filp->f_mapping = inode->i_mapping;
+ filp->f_path = nd.path;
+ filp->f_pos = 0;
+ filp->f_op = NULL;
+ file_move(filp, &inode->i_sb->s_files);
+
+ return filp;
+
+out_put_filp:
+ put_filp(filp);
+out_err:
+ return ERR_PTR(err);
+}
+
/*
* Note that the low bits of the passed in "open_flag"
* are not the same as in the local variable "flag". See
@@ -1668,6 +1700,9 @@ struct file *do_filp_open(int dfd, const
int will_write;
int flag = open_to_namei_flags(open_flag);
+ if (flag & O_NOACC)
+ return open_noaccess(dfd, pathname, open_flag);
+
if (!acc_mode)
acc_mode = MAY_OPEN | ACC_MODE(flag);
Index: linux-2.6/include/asm-generic/fcntl.h
===================================================================
--- linux-2.6.orig/include/asm-generic/fcntl.h 2009-05-20 14:12:00.000000000 +0200
+++ linux-2.6/include/asm-generic/fcntl.h 2009-06-18 07:16:29.000000000 +0200
@@ -51,6 +51,9 @@
#ifndef O_CLOEXEC
#define O_CLOEXEC 02000000 /* set close_on_exec */
#endif
+#ifndef O_NOACC
+#define O_NOACC 04000000 /* open without any access */
+#endif
#ifndef O_NDELAY
#define O_NDELAY O_NONBLOCK
#endif
On Thu, Jun 18, 2009 at 08:55:37AM +0200, Miklos Szeredi wrote:
> Implement a new open flag that doesn't require any access on the file.
> It works on any file type including symlinks.
>
> The sole purpose is to help race free "userspace lookup" type
> operations. So fstat, fch*, *at work but nothing else. Filesystem's
> ->open() is not called and f_op is set to NULL.
we guarantee that f_op is never NULL, so you'll need to assign a
file operations structure that is empty to it to avoid crashed in
various places.
> It would be logical to reuse the open_flag=3 value, but that has
> historically been used with different semantics so I'm afraid of
> touching it.
I think the historical semantics are exactly that you can open it
an issue ioctls + stat / etc on it ut not actually read/write it.
On Tue, 23 Jun 2009, Christoph Hellwig wrote:
> we guarantee that f_op is never NULL, so you'll need to assign a
> file operations structure that is empty to it to avoid crashed in
> various places.
Yeah, the patch was just a proof of concept thing (and most places
still check f_op != NULL before dereferncing, so..)
> > It would be logical to reuse the open_flag=3 value, but that has
> > historically been used with different semantics so I'm afraid of
> > touching it.
>
> I think the historical semantics are exactly that you can open it
> an issue ioctls + stat / etc on it ut not actually read/write it.
Two differences between open("foo", 3) and open("foo", O_NOACC):
1) open with "3" requires _read_and_write_ permissions on foo, but
does not allow either read or write. Not sure what the logic in
that, but that's the way it has always been.
2) open with "3" calls driver's ->open() with any side effect that
may have. Open with O_NOACC doesn't do that, and hence if we
want to allow ioctls they need a new interface which gets a
"struct path" instead of a "struct file".
Thanks,
Miklos
On Tue, Jun 23, 2009 at 04:12:22PM +0200, Miklos Szeredi wrote:
> > an issue ioctls + stat / etc on it ut not actually read/write it.
>
> Two differences between open("foo", 3) and open("foo", O_NOACC):
>
> 1) open with "3" requires _read_and_write_ permissions on foo, but
> does not allow either read or write. Not sure what the logic in
> that, but that's the way it has always been.
Which is a quite sensible requirement if we want to do ioctls.
>
> 2) open with "3" calls driver's ->open() with any side effect that
> may have. Open with O_NOACC doesn't do that, and hence if we
> want to allow ioctls they need a new interface which gets a
> "struct path" instead of a "struct file".
Well, we'll need ->open to support ioctls, and I think it's good to go
down that road.
On Tue, 23 Jun 2009, Christoph Hellwig wrote:
> On Tue, Jun 23, 2009 at 04:12:22PM +0200, Miklos Szeredi wrote:
> > > an issue ioctls + stat / etc on it ut not actually read/write it.
> >
> > Two differences between open("foo", 3) and open("foo", O_NOACC):
> >
> > 1) open with "3" requires _read_and_write_ permissions on foo, but
> > does not allow either read or write. Not sure what the logic in
> > that, but that's the way it has always been.
>
> Which is a quite sensible requirement if we want to do ioctls.
Right, but that makes it useless for things which want to open it
despite having no permission on the file. For example some programs
do:
fd = open(".", 0);
chdir(someplace);
/* do something */
fchdir(fd);
or for that matter:
fd = open("foo", 0);
fstat(fd, &stat1);
if (something)
fchmod(fd, ...);
close(fd);
Note: neither chmod() nor chdir() (nor host of others) require either
read or write permissions on the file, so we _should_ be able to do
this kind of thing without open() requiring them.
Thanks,
Miklos
Miklos Szeredi <[email protected]> wrote:
> Right, but that makes it useless for things which want to open it
> despite having no permission on the file.
For my purposes, if I'm going to emulate pioctl() in userspace, I also need to
be able to open device files that don't have drivers available, and when you
do open a dev file in this manner, it must _not_ call the ->open() routine of
the device driver.
David
On Tue, 23 Jun 2009 16:52:17 +0100
David Howells <[email protected]> wrote:
> Miklos Szeredi <[email protected]> wrote:
>
> > Right, but that makes it useless for things which want to open it
> > despite having no permission on the file.
>
> For my purposes, if I'm going to emulate pioctl() in userspace, I also need to
> be able to open device files that don't have drivers available, and when you
> do open a dev file in this manner, it must _not_ call the ->open() routine of
> the device driver
So how are you going to make that work in conjunction with the in
progress work on doing stuff like revoke(), and with module reference
counting ?
Alan Cox <[email protected]> wrote:
> So how are you going to make that work in conjunction with the in
> progress work on doing stuff like revoke(), and with module reference
> counting ?
Reference count on which module? This would not take a reference on the device
driver as it would not refer to it, but would still have a file struct, an
inode struct and a dentry struct on the underlying fs.
David
On Tue, 23 Jun 2009, David Howells wrote:
> Reference count on which module? This would not take a reference on
> the device driver as it would not refer to it, but would still have
> a file struct, an inode struct and a dentry struct on the underlying
> fs.
So how about the following: provide a new open flag O_FILESYSTEM,
meaning it opens the file on the underlying filesystem instead of the
device/socket/symlink/etc...
Add a new inode->i_filesystem_fop pointer which is used instead of
inode->i_fop in case if O_FILESYSTEM. If ->i_filesystem_fop isn't set
by the filesystem, fall back to a default which returns -EBADF for all
operations (except open).
Define O_NOACC as 3. On open(..., O_FILESYSTEM | O_NOACC) require no
privileges on the file.
AFS would set up i_filesystem_fop with its ->ioctl() function. No
special handling needed for revoke()...
That would work, no?
Thanks,
Miklos
On Tue, 23 Jun 2009 17:10:52 +0100
David Howells <[email protected]> wrote:
> Alan Cox <[email protected]> wrote:
>
> > So how are you going to make that work in conjunction with the in
> > progress work on doing stuff like revoke(), and with module reference
> > counting ?
>
> Reference count on which module? This would not take a reference on the device
> driver as it would not refer to it, but would still have a file struct, an
> inode struct and a dentry struct on the underlying fs.
Which means the device could be unloaded, something else loaded and your
handle wouldn't be invalidated but would be stale ?
Perhaps it would make more sense to me if I knew why you needed to do
this ?
On Tue, Jun 23, 2009 at 04:52:17PM +0100, David Howells wrote:
> Miklos Szeredi <[email protected]> wrote:
>
> > Right, but that makes it useless for things which want to open it
> > despite having no permission on the file.
>
> For my purposes, if I'm going to emulate pioctl() in userspace, I also need to
> be able to open device files that don't have drivers available, and when you
> do open a dev file in this manner, it must _not_ call the ->open() routine of
> the device driver.
Last time I checked afs didn't support device files at all. And limited
the magic commands to not work on device files doesn't sound to bad.
Of course specifying what these magic tools are actually supposed to do
would be an interesting start.
Miklos Szeredi <[email protected]> wrote:
> Not sure if it's a good base for what AFS/pioctl is trying to do, but
> it's something that I personally would very much like to see.
It could be useful. The problem is that I then need to do an ioctl() on it.
To emulate a pioctl with this, I'd need something like the following in a
library:
long pioctl(char *path, int cmd, struct ViceIoctl *arg, int nofollow)
{
struct ioctl_pioctl {
int cmd;
struct ViceIoctl *arg;
} ip;
long ret;
int fd;
switch (cmd) {
case VIOC_CMD1:
case VIOC_CMD2:
case VIOC_CMD3:
fd = open(path, O_NOACC | (nofollow ? O_NOFOLLOW : 0));
ip.cmd = cmd;
ip.arg = arg;
ret = ioctl(fd, DO_PIOCTL, &ip);
close(fd);
break;
}
return ret;
}
Now, the DO_PIOCTL constant could be dispensed with, and each VIOC_CMDx be
mapped to a different ioctl command number. Note that you may not assume that
the set of pioctl numbers shares with the set of ioctl numbers, so you can't
just hand the cmd number through as-is.
Not all pioctl commands need to be emulated this way. Some could call getxattr
instead, for instance, and some could muck around with /proc and /sys files.
> 2) open with "3" calls driver's ->open() with any side effect that
> may have. Open with O_NOACC doesn't do that, and hence if we
> want to allow ioctls they need a new interface which gets a
> "struct path" instead of a "struct file".
You don't actually need that. struct file contains a struct path. You just
need a way to come up with an alternative set of file ops for an O_NOACC open
- perhaps through struct inode_operations.
Alternatively, you could install a special set of generic file ops that only
has an ioctl op. This could then redirect through another ioctl op in the
inode_operations struct.
As I mentioned in another email, for emulation of pioctl(), I need O_NOACC to
_not_ call device_driver->open() on device files, and to not install the
device driver file ops table. pioctl() operates on the underlying file in the
underlying fs.
David
Miklos Szeredi <[email protected]> wrote:
> So how about the following: provide a new open flag O_FILESYSTEM,
> meaning it opens the file on the underlying filesystem instead of the
> device/socket/symlink/etc...
Yes. That's what I need. That's what pioctl() is for: it operates on the
underlying fs, not any special aspects of the various types of file, and since
it doesn't do traditional I/O on those files, it doesn't need to, and
shouldn't, open them (thus avoiding side effects from ->open()), and doesn't
need R/W access to them.
> Add a new inode->i_filesystem_fop pointer
I'd rather not put it there. That means the inode struct grows. Perhaps
attach it to the inode_operations table or stick an open_noaccess() op in the
iops table.
> Define O_NOACC as 3. On open(..., O_FILESYSTEM | O_NOACC) require no
> privileges on the file.
It must also work with O_NOFOLLOW, which I think your suggestion will.
> AFS would set up i_filesystem_fop with its ->ioctl() function. No
> special handling needed for revoke()...
Sounds reasonable.
> That would work, no?
I think so.
David
Alan Cox <[email protected]> wrote:
> Which means the device could be unloaded, something else loaded and your
> handle wouldn't be invalidated but would be stale ?
It would not be stale. The O_NOACC handle would effectively be a route to the
inode on the underlying fs, and so shouldn't affect normal opens that do get
routed to the device driver.
However, Miklos's O_FILESYSTEM suggestion would work as a differentiation to
O_NOACC, if you want the latter to go to the device driver.
> Perhaps it would make more sense to me if I knew why you needed to do this ?
How about I give you an example:
There's a pioctl to kick a file out of the local cache and reload it. AFS
symlinks are stored in the local cache, and so you might conceivably want to
eject one from the local cache on your machine. This is an operation on the
symlink object itself, _not_ on the target of the symlink.
However, unless I can open a symlink without it giving me an error or trying
to open the target, I cannot issue an ioctl() upon it.
Also, this is a command, not an attribute, and so using lsetxattr() is a bit
of a violation of semantics, and, furthermore, it's not an attribute of the
file.
Now, consider that I may, at some point in the future, or in some other netfs,
need to deal with device files. I may want to eject a dev file from the
cache; I might want to lock one into the cache. I need to affect the
filesystem object underlying the device file, not the device driver's opinion
of the device file. The device driver knows nothing about the filesystem's
local caching, and shouldn't have to deal with it. I need a way to operate on
a device file without (a) incurring ->open() side effects of a dev file, (b)
pestering the device driver, (c) incurring a failure because the dev file
doesn't have a corresponding driver, or (d) requiring permission to open that
dev file.
It is probably worth adding an fscachectl() syscall to handle caching commands,
since these are fairly simple and need to be applied to NFS as well as AFS, but
there are other file commands too (not just things that get attributes). See:
http://www.openafs.org/cgi-bin/cvsweb.cgi/openafs/doc/pdf/fscm-ispec.pdf?rev=1.1&content-type=text/x-cvsweb-markup
for more information.
David
On Tue, 23 Jun 2009, David Howells wrote:
> Miklos Szeredi <[email protected]> wrote:
>
> > Define O_NOACC as 3. On open(..., O_FILESYSTEM | O_NOACC) require no
> > privileges on the file.
>
> It must also work with O_NOFOLLOW, which I think your suggestion will.
This does sound like a fairly natural extension of what we already do.
We essentially already have O_NOACCESS (3), and use it exactly because we
need to do operations on a file descriptor without "real" accesses
(notably things like accessing /dev/cdrom without waiting/checking for the
disk being present etc).
O_FILESYSTEM I don't like as a name (to me, it doesn't say _what_ it is
doing - of course an open works on a filesystem!), but the concept of
saying "don't follow device nodes - just open the node itself" makes
perfect sense. Together with O_NOFOLLOW it also fairly naturally means
"give me the actual symlink _node_, don't return error or follow it".
And we can trivially test at a higher level that O_FILESYSTEM (with a
better name, please), is always paired with O_NOACCESS (not O_NOACC: we do
not try to save three letters, there is no shortage). Because the raw node
obviously must never really be "accessed" (ie you can't do read/write etc
on it).
That said, I do _not_ like the notion of
> Add a new inode->i_filesystem_fop pointer
regardless of whether it's in inode->i_op or wherever. I think we should
just handle this in the regular "inode->f_op->open" routine, the same way
we handle FMODE_EXCLUSIVE (O_EXCL), FMODE_NDELAY (O_NONBLOCK) and lack of
access rights (O_NOACCESS) in the driver open routines that currently
handle those specially (O_NDELAY is spe
Al?
Linus
On Tue, 23 Jun 2009, Linus Torvalds wrote:
> On Tue, 23 Jun 2009, David Howells wrote:
>
> > Miklos Szeredi <[email protected]> wrote:
> >
> > > Define O_NOACC as 3. On open(..., O_FILESYSTEM | O_NOACC) require no
> > > privileges on the file.
> >
> > It must also work with O_NOFOLLOW, which I think your suggestion will.
>
> This does sound like a fairly natural extension of what we already do.
>
> We essentially already have O_NOACCESS (3), and use it exactly because we
> need to do operations on a file descriptor without "real" accesses
> (notably things like accessing /dev/cdrom without waiting/checking for the
> disk being present etc).
>
> O_FILESYSTEM I don't like as a name (to me, it doesn't say _what_ it is
> doing - of course an open works on a filesystem!), but the concept of
> saying "don't follow device nodes - just open the node itself" makes
> perfect sense. Together with O_NOFOLLOW it also fairly naturally means
> "give me the actual symlink _node_, don't return error or follow it".
O_NODEV? It applies just as well to fifos, sockets and symlinks, but
it's hard to express that in a compact way.
> That said, I do _not_ like the notion of
>
> > Add a new inode->i_filesystem_fop pointer
>
> regardless of whether it's in inode->i_op or wherever. I think we should
> just handle this in the regular "inode->f_op->open" routine, the same way
No, it's a totally different open, one comes from the device/fifo
code, the other from the filesystem. Yes, the filesystem could
in theory wedge itself between the VFS and device's f_ops. Not
sure if that's how this should be done, though...
Also how should the default case (filesystem doesn't handle
O_NODEV) be handled. The nice thing about O_NODEV | O_NOACCESS
would be that it could be implemented totally in generic code in
a secure way and it would even be useful for a variety of
cases.
Thanks,
Miklos
Linus Torvalds <[email protected]> wrote:
> That said, I do _not_ like the notion of
>
> > Add a new inode->i_filesystem_fop pointer
>
> regardless of whether it's in inode->i_op or wherever. I think we should
> just handle this in the regular "inode->f_op->open" routine, the same way
> we handle FMODE_EXCLUSIVE (O_EXCL), FMODE_NDELAY (O_NONBLOCK) and lack of
> access rights (O_NOACCESS) in the driver open routines that currently
> handle those specially (O_NDELAY is spe
Did you accidentally delete more than the word 'special' there?
David
On Tue, Jun 23, 2009 at 10:34:49AM -0700, Linus Torvalds wrote:
> That said, I do _not_ like the notion of
>
> > Add a new inode->i_filesystem_fop pointer
>
> regardless of whether it's in inode->i_op or wherever. I think we should
> just handle this in the regular "inode->f_op->open" routine, the same way
> we handle FMODE_EXCLUSIVE (O_EXCL), FMODE_NDELAY (O_NONBLOCK) and lack of
> access rights (O_NOACCESS) in the driver open routines that currently
> handle those specially (O_NDELAY is spe
>
> Al?
i_filesystem_fop is certainly bogus, but why do we want to bother with
file_operations at all?
Unless you really insist on unlimited use of ioctl(2) on such beasts (and
any users will be non-portable for obvious reasons anyway), there's no need
to go anywhere near ->open() *or* ->f_op in general.
Just add new methods to ->i_op (and we already have that coming from
fs code) and teach do_filp_open() to
* call permission() with new flag (MAY_TALK_TO_FS_NODE) for such
open()
* do not die with -ELOOP on symlinks if we have O_NOFOLLOW + your flag
* do not call ->f_op->open() at all for such open()
and we are all set. Hell, we can even teach sys_ioctl() that given set
of ioctls maps to calls of our new methods. Taken from ->i_op...
If we want full-blown ->ioctl() coming from the fs code on such opens, we
will need distinct file_operations, no matter what we do with ->open().
It's also doable (we'd need ->i_fop pointing to new foofs_special_file_ops
and its ->open() to be a boilerplate that would replace file->f_op with
the normal one in case of normal open()), but it's more boilerplate patches
and I really don't see what would it buy...
Comments?
On Wed, 24 Jun 2009, Al Viro wrote:
> i_filesystem_fop is certainly bogus, but why do we want to bother with
> file_operations at all?
>
> Unless you really insist on unlimited use of ioctl(2) on such beasts (and
> any users will be non-portable for obvious reasons anyway), there's no need
> to go anywhere near ->open() *or* ->f_op in general.
>
> Just add new methods to ->i_op (and we already have that coming from
> fs code) and teach do_filp_open() to
> * call permission() with new flag (MAY_TALK_TO_FS_NODE) for such
> open()
> * do not die with -ELOOP on symlinks if we have O_NOFOLLOW + your flag
> * do not call ->f_op->open() at all for such open()
> and we are all set.
Sounds good.
> Hell, we can even teach sys_ioctl() that given set
> of ioctls maps to calls of our new methods. Taken from ->i_op...
>
> If we want full-blown ->ioctl() coming from the fs code on such opens, we
> will need distinct file_operations, no matter what we do with ->open().
> It's also doable (we'd need ->i_fop pointing to new foofs_special_file_ops
> and its ->open() to be a boilerplate that would replace file->f_op with
> the normal one in case of normal open()), but it's more boilerplate patches
> and I really don't see what would it buy...
Adding boilerplate to _every_ filesystem sounds really fragile and
stupid. At least add a filesystem (or inode) flag:
FS_HANDLES_NODE_OPENS and otherwise don't call into filesystem's
->open().
Thanks,
Miklos
On Wed, 24 Jun 2009, Al Viro wrote:
>
> i_filesystem_fop is certainly bogus, but why do we want to bother with
> file_operations at all?
>
> Unless you really insist on unlimited use of ioctl(2) on such beasts (and
> any users will be non-portable for obvious reasons anyway), there's no need
> to go anywhere near ->open() *or* ->f_op in general.
A lot of filesystems (especially network filesystems) want to do something
special when you open a node on them.
NFS, for example, does that whole alloc_nfs_open_context() thing to keep
track of RPC credentials etc. It's where things like "filp->f_private"
get set etc.
So if you don't call open(), you'll not initialize the filp sufficiently
to do lots of operations.
But yes:
> Just add new methods to ->i_op (and we already have that coming from
> fs code) and teach do_filp_open() to
> * call permission() with new flag (MAY_TALK_TO_FS_NODE) for such
> open()
> * do not die with -ELOOP on symlinks if we have O_NOFOLLOW + your flag
> * do not call ->f_op->open() at all for such open()
> and we are all set. Hell, we can even teach sys_ioctl() that given set
> of ioctls maps to calls of our new methods. Taken from ->i_op...
Sure. That will work, but I do think that it's going to be more hacky than
just trying to make the file descriptor look as real as possible, and just
calling "open" on it.
But I don't really have any strong opinions.
Linus
On Wed, Jun 17, 2009 at 23:55, Miklos Szeredi<[email protected]> wrote:
> The sole purpose is to help race free "userspace lookup" type
> operations. So fstat, fch*, *at work but nothing else. Filesystem's
> ->open() is not called and f_op is set to NULL.
Before adding something like this, please look at the 2009 POSIX spec.
There are two new open() flags which have to be implemented: O_EXEC
and O_SEARCH. These might already suffice for what you want.
On Wed, 24 Jun 2009, Ulrich Drepper wrote:
> On Wed, Jun 17, 2009 at 23:55, Miklos Szeredi<[email protected]> wrote:
> > The sole purpose is to help race free "userspace lookup" type
> > operations. So fstat, fch*, *at work but nothing else. Filesystem's
> > ->open() is not called and f_op is set to NULL.
>
> Before adding something like this, please look at the 2009 POSIX spec.
Is it available online somewhere? If not, could you please post the
relevant parts?
> There are two new open() flags which have to be implemented: O_EXEC
> and O_SEARCH. These might already suffice for what you want.
I suspect they are not quite what we want (both of them requring
MAY_EXEC on file, no?)
Thanks,
Miklos
On Wed, Jun 24, 2009 at 11:06, Miklos Szeredi<[email protected]> wrote:
> Is it available online somewhere? If not, could you please post the
> relevant parts?
The OpenGroup publishes the man pages. Here is what google finds:
http://www.opengroup.org/onlinepubs/9699919799/functions/open.html
> I suspect they are not quite what we want (both of them requring
> MAY_EXEC on file, no?)
No that's the differences between O_EXEC and O_SEARCH. We
deliberately introduced two different names even though some OSes
might implement it using the same flag and even though in both cases
irritatingly the x permission bit is used.