Prev: [RFC] perf migration
Next: ocfs2: When zero extending, do it by page.
From: Nick Piggin on 7 Jul 2010 14:30
On Wed, Jul 07, 2010 at 11:23:31PM +0530, Aneesh Kumar K. V wrote:
> On Thu, 8 Jul 2010 02:57:19 +1000, Nick Piggin <npiggin(a)suse.de> wrote:
> > I haven't followed this thread closely, so can you just go back
> > and explain to me why you need this? Ie. that you can't achieve with
> > a path-to-handle follow/nofollow option.
>
>
> We need to be able to read the link target using file handle. The exact
> usecase i have is with respect to implementing READLINK operation on a
> userspace NFS server. The request contain the file handle and the
> response include target name.

Fair point, could you include this stuff in your changelog? It may
help with other people reviewing it too who have not followed closely.


> Similarly we need to able to get file attribute based on file handle.
> I was using readlinkat and fstat to obtain both the above information
> using the descriptor returned by open_by_handle on a file handle
> representing symlink
>
> -aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Aneesh Kumar K. V on 8 Jul 2010 06:50
On Thu, 8 Jul 2010 02:48:59 +1000, Nick Piggin <npiggin(a)suse.de> wrote:
> On Tue, Jun 15, 2010 at 10:42:54PM +0530, Aneesh Kumar K.V wrote:
> > The patch update may_open to allow handle based open on symlinks.
> > The file handle based API use file descritor returned from open_by_handle_at
> > to do different file system operations. To find the link target name we
> > need to get a file descriptor on symlinks.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>
> > ---
> > fs/namei.c | 17 ++++++++++++++---
> > 1 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index c2d19c7..417bf53 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -1422,7 +1422,7 @@ int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
> > return error;
> > }
> >
> > -int may_open(struct path *path, int acc_mode, int flag)
> > +static int __may_open(struct path *path, int acc_mode, int flag, int handle)
> > {
> > struct dentry *dentry = path->dentry;
> > struct inode *inode = dentry->d_inode;
> > @@ -1433,7 +1433,13 @@ int may_open(struct path *path, int acc_mode, int flag)
> >
> > switch (inode->i_mode & S_IFMT) {
> > case S_IFLNK:
> > - return -ELOOP;
> > + /*
> > + * For file handle based open we should allow
> > + * open of symlink.
> > + */
> > + if (!handle)
> > + return -ELOOP;
> > + break;
> > case S_IFDIR:
> > if (acc_mode & MAY_WRITE)
> > return -EISDIR;
> > @@ -1473,6 +1479,11 @@ int may_open(struct path *path, int acc_mode, int flag)
> > return break_lease(inode, flag);
> > }
> >
> > +int may_open(struct path *path, int acc_mode, int flag)
> > +{
> > + return __may_open(path, acc_mode, flag, 0);
> > +}
>
> Why don't you just add MAY_OPEN_LNK instead of this ugliness?
>

Something like the below ?

commit 69787f5ed6c8a7a63ad21ef5d7de0b62f124ff0a
Author: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>
Date: Tue Jul 6 23:25:57 2010 +0530

vfs: Allow handle based open on symlinks

The patch update may_open to allow handle based open on symlinks.
The file handle based API use file descritor returned from open_by_handle_at
to do different file system operations. To find the link target name we
need to get a file descriptor on symlinks.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>

diff --git a/fs/namei.c b/fs/namei.c
index de40312..2ea5c79 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1456,7 +1456,15 @@ int may_open(struct path *path, int acc_mode, int flag)

switch (inode->i_mode & S_IFMT) {
case S_IFLNK:
- return -ELOOP;
+ /*
+ * Allow only if acc_mode contain
+ * open link request and read request.
+ */
+ if (acc_mode != (MAY_OPEN_LINK | MAY_READ))
+ return -ELOOP;
+ if (flag != O_RDONLY)
+ return -ELOOP;
+ break;
case S_IFDIR:
if (acc_mode & MAY_WRITE)
return -EISDIR;
diff --git a/fs/open.c b/fs/open.c
index daf2534..a58837a 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1266,8 +1266,15 @@ static long do_sys_open_by_handle(int mountdirfd,
*/
if (open_flag & __O_SYNC)
open_flag |= O_DSYNC;
+ /*
+ * Handle based API allow open on a symlink
+ */
+ if (S_ISLNK(path->dentry->d_inode->i_mode))
+ acc_mode = MAY_OPEN_LINK;
+ else
+ acc_mode = MAY_OPEN;

- acc_mode = MAY_OPEN | ACC_MODE(open_flag);
+ acc_mode |= ACC_MODE(open_flag);

/* O_TRUNC implies we need access checks for write permissions */
if (open_flag & O_TRUNC)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index cbff4ca..eaf13d1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -53,6 +53,7 @@ struct inodes_stat_t {
#define MAY_APPEND 8
#define MAY_ACCESS 16
#define MAY_OPEN 32
+#define MAY_OPEN_LINK 64

/*
* flags in file.f_mode. Note that FMODE_READ and FMODE_WRITE must correspond
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Miklos Szeredi on 12 Jul 2010 04:30
On Mon, 12 Jul 2010, Aneesh Kumar K.V wrote:
> The patch update may_open to allow handle based open on symlinks.
> The file handle based API use file descritor returned from open_by_handle_at
> to do different file system operations. To find the link target name we
> need to get a file descriptor on symlinks.
>
> We should be able to read the link target using file handle. The exact
> usecase is with respect to implementing READLINK operation on a
> userspace NFS server. The request contain the file handle and the
> response include target name.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>
> ---
> fs/namei.c | 10 +++++++++-
> fs/open.c | 9 ++++++++-
> include/linux/fs.h | 1 +
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 4d590a3..a6a8093 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1456,7 +1456,15 @@ int may_open(struct path *path, int acc_mode, int flag)
>
> switch (inode->i_mode & S_IFMT) {
> case S_IFLNK:
> - return -ELOOP;
> + /*
> + * Allow only if acc_mode contain
> + * open link request and read request.
> + */
> + if (acc_mode != (MAY_OPEN_LINK | MAY_READ))

Why require MAY_READ?

Actually, open_by_handle() should be a good place to start supporting
O_NOACCESS from the start. I.e. neigher read, nor write access is
permitted on the file.


> + return -ELOOP;
> + if (flag != O_RDONLY)
> + return -ELOOP;
> + break;
> case S_IFDIR:
> if (acc_mode & MAY_WRITE)
> return -EISDIR;
> diff --git a/fs/open.c b/fs/open.c
> index df5d21e..afb089e 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1268,8 +1268,15 @@ static long do_sys_open_by_handle(int mountdirfd,
> */
> if (open_flag & __O_SYNC)
> open_flag |= O_DSYNC;
> + /*
> + * Handle based API allow open on a symlink
> + */
> + if (S_ISLNK(path->dentry->d_inode->i_mode))
> + acc_mode = MAY_OPEN_LINK;
> + else
> + acc_mode = MAY_OPEN;
>
> - acc_mode = MAY_OPEN | ACC_MODE(open_flag);
> + acc_mode |= ACC_MODE(open_flag);
>
> /* O_TRUNC implies we need access checks for write permissions */
> if (open_flag & O_TRUNC)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index a458b4e..08afa72 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -53,6 +53,7 @@ struct inodes_stat_t {
> #define MAY_APPEND 8
> #define MAY_ACCESS 16
> #define MAY_OPEN 32
> +#define MAY_OPEN_LINK 64
>
> /*
> * flags in file.f_mode. Note that FMODE_READ and FMODE_WRITE must correspond
> --
> 1.7.2.rc1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Aneesh Kumar K. V on 12 Jul 2010 05:50
On Mon, 12 Jul 2010 10:23:21 +0200, Miklos Szeredi <miklos(a)szeredi.hu> wrote:
> On Mon, 12 Jul 2010, Aneesh Kumar K.V wrote:
> > The patch update may_open to allow handle based open on symlinks.
> > The file handle based API use file descritor returned from open_by_handle_at
> > to do different file system operations. To find the link target name we
> > need to get a file descriptor on symlinks.
> >
> > We should be able to read the link target using file handle. The exact
> > usecase is with respect to implementing READLINK operation on a
> > userspace NFS server. The request contain the file handle and the
> > response include target name.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>
> > ---
> > fs/namei.c | 10 +++++++++-
> > fs/open.c | 9 ++++++++-
> > include/linux/fs.h | 1 +
> > 3 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 4d590a3..a6a8093 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -1456,7 +1456,15 @@ int may_open(struct path *path, int acc_mode, int flag)
> >
> > switch (inode->i_mode & S_IFMT) {
> > case S_IFLNK:
> > - return -ELOOP;
> > + /*
> > + * Allow only if acc_mode contain
> > + * open link request and read request.
> > + */
> > + if (acc_mode != (MAY_OPEN_LINK | MAY_READ))
>
> Why require MAY_READ?

a value of 0x0 for flag in open(2) implies a read ?

>
> Actually, open_by_handle() should be a good place to start supporting
> O_NOACCESS from the start. I.e. neigher read, nor write access is
> permitted on the file.

Yes that would be ideal. But that will include much larger change. I was
hoping we could get something in this merge window with the change
suggested above ?

>
>
> > + return -ELOOP;
> > + if (flag != O_RDONLY)
> > + return -ELOOP;
> > + break;
> > case S_IFDIR:
> > if (acc_mode & MAY_WRITE)
> > return -EISDIR;

-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Miklos Szeredi on 12 Jul 2010 13:00
On Mon, 12 Jul 2010, Aneesh Kumar K. V wrote:
> On Mon, 12 Jul 2010 10:23:21 +0200, Miklos Szeredi <miklos(a)szeredi.hu> wrote:
> > On Mon, 12 Jul 2010, Aneesh Kumar K.V wrote:
> > > The patch update may_open to allow handle based open on symlinks.
> > > The file handle based API use file descritor returned from open_by_handle_at
> > > to do different file system operations. To find the link target name we
> > > need to get a file descriptor on symlinks.
> > >
> > > We should be able to read the link target using file handle. The exact
> > > usecase is with respect to implementing READLINK operation on a
> > > userspace NFS server. The request contain the file handle and the
> > > response include target name.
> > >
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com>
> > > ---
> > > fs/namei.c | 10 +++++++++-
> > > fs/open.c | 9 ++++++++-
> > > include/linux/fs.h | 1 +
> > > 3 files changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/namei.c b/fs/namei.c
> > > index 4d590a3..a6a8093 100644
> > > --- a/fs/namei.c
> > > +++ b/fs/namei.c
> > > @@ -1456,7 +1456,15 @@ int may_open(struct path *path, int acc_mode, int flag)
> > >
> > > switch (inode->i_mode & S_IFMT) {
> > > case S_IFLNK:
> > > - return -ELOOP;
> > > + /*
> > > + * Allow only if acc_mode contain
> > > + * open link request and read request.
> > > + */
> > > + if (acc_mode != (MAY_OPEN_LINK | MAY_READ))
> >
> > Why require MAY_READ?
>
> a value of 0x0 for flag in open(2) implies a read ?

Yes.

However a value of 0x3 is not defined in POSIX, and in linux it's a
sort of weird O_NOACCESS: it requires both read and write permission
on the file but allows neither read or write.

Requiring permission is because on device files the open can have side
effects. Not sure if open_by_handle() really wants to allow device
opens, that's another thing to think about.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2
Prev: [RFC] perf migration
Next: ocfs2: When zero extending, do it by page.