[patch] F_GETPATH for linux

[patch] F_GETPATH for linux

[Davide Libenzi]

I was working on some bsd/linux compatibility code, and I noticed that we 
do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
Here it is, building but untested ...



- Davide


---
 fs/fcntl.c                  |   27 +++++++++++++++++++++++++++
 fs/proc/base.c              |   26 +-------------------------
 include/asm-generic/fcntl.h |    1 +
 include/linux/fs.h          |    1 +
 4 files changed, 30 insertions(+), 25 deletions(-)

Index: linux-2.6.mod/fs/fcntl.c
===================================================================
--- linux-2.6.mod.orig/fs/fcntl.c	2008-04-13 14:17:39.000000000 -0700
+++ linux-2.6.mod/fs/fcntl.c	2008-04-13 14:54:00.000000000 -0700
@@ -316,6 +316,29 @@
 	return pid;
 }
 
+int fcntl_getpath(struct path *fpath, char __user *path, int maxsize)
+{
+	int size;
+	char *buf, *pathname;
+
+	buf = (char *) __get_free_page(GFP_TEMPORARY);
+	if (!buf)
+		return -ENOMEM;
+	pathname = d_path(fpath, buf, PAGE_SIZE);
+	size = PTR_ERR(pathname);
+	if (IS_ERR(pathname))
+		goto error;
+	size = buf + PAGE_SIZE - 1 - pathname;
+	if (size > maxsize)
+		size = maxsize;
+	if (copy_to_user(path, pathname, size))
+		size = -EFAULT;
+error:
+	free_page((unsigned long) buf);
+	return size;
+}
+EXPORT_SYMBOL(fcntl_getpath);
+
 static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
 		struct file *filp)
 {
@@ -381,6 +404,10 @@
 	case F_NOTIFY:
 		err = fcntl_dirnotify(fd, filp, arg);
 		break;
+	case F_GETPATH:
+		err = fcntl_getpath(&filp->f_path, (char __user *) arg,
+				    PATH_MAX);
+		break;
 	default:
 		break;
 	}
Index: linux-2.6.mod/include/asm-generic/fcntl.h
===================================================================
--- linux-2.6.mod.orig/include/asm-generic/fcntl.h	2008-04-13 14:17:33.000000000 -0700
+++ linux-2.6.mod/include/asm-generic/fcntl.h	2008-04-13 14:18:17.000000000 -0700
@@ -73,6 +73,7 @@
 #define F_SETSIG	10	/* for sockets. */
 #define F_GETSIG	11	/* for sockets. */
 #endif
+#define F_GETPATH	12
 
 /* for F_[GET|SET]FL */
 #define FD_CLOEXEC	1	/* actually anything with low bit set goes */
Index: linux-2.6.mod/fs/proc/base.c
===================================================================
--- linux-2.6.mod.orig/fs/proc/base.c	2008-04-13 14:41:03.000000000 -0700
+++ linux-2.6.mod/fs/proc/base.c	2008-04-13 14:42:12.000000000 -0700
@@ -1192,30 +1192,6 @@
 	return ERR_PTR(error);
 }
 
-static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
-{
-	char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
-	char *pathname;
-	int len;
-
-	if (!tmp)
-		return -ENOMEM;
-
-	pathname = d_path(path, tmp, PAGE_SIZE);
-	len = PTR_ERR(pathname);
-	if (IS_ERR(pathname))
-		goto out;
-	len = tmp + PAGE_SIZE - 1 - pathname;
-
-	if (len > buflen)
-		len = buflen;
-	if (copy_to_user(buffer, pathname, len))
-		len = -EFAULT;
- out:
-	free_page((unsigned long)tmp);
-	return len;
-}
-
 static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int buflen)
 {
 	int error = -EACCES;
@@ -1230,7 +1206,7 @@
 	if (error)
 		goto out;
 
-	error = do_proc_readlink(&path, buffer, buflen);
+	error = fcntl_getpath(&path, buffer, buflen);
 	path_put(&path);
 out:
 	return error;
Index: linux-2.6.mod/include/linux/fs.h
===================================================================
--- linux-2.6.mod.orig/include/linux/fs.h	2008-04-13 14:40:01.000000000 -0700
+++ linux-2.6.mod/include/linux/fs.h	2008-04-13 14:40:34.000000000 -0700
@@ -960,6 +960,7 @@
 /* only for net: no internal synchronization */
 extern void __kill_fasync(struct fasync_struct *, int, int);
 
+extern int fcntl_getpath(struct path *fpath, char __user *path, int maxsize);
 extern int __f_setown(struct file *filp, struct pid *, enum pid_type, int force);
 extern int f_setown(struct file *filp, unsigned long arg, int force);
 extern void f_delown(struct file *filp);

Re: [patch] F_GETPATH for linux

[Arnd Bergmann]

On Monday 14 April 2008, Davide Libenzi wrote:
> I was working on some bsd/linux compatibility code, and I noticed that we 
> do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
> what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
> Here it is, building but untested ...
 
I reviewed the code path for compat_sys_fcntl{,64} for this patch, looks
good in that respect.

> +error:
> +	free_page((unsigned long) buf);
> +	return size;
> +}
> +EXPORT_SYMBOL(fcntl_getpath);
> +

Procfs cannot be a module, and there are no other users of this function,
so why do you export it?

	Arnd <><

Re: [patch] F_GETPATH for linux

[Davide Libenzi]

On Mon, 14 Apr 2008, Arnd Bergmann wrote:

> On Monday 14 April 2008, Davide Libenzi wrote:
> > I was working on some bsd/linux compatibility code, and I noticed that we 
> > do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
> > what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
> > Here it is, building but untested ...
>  
> I reviewed the code path for compat_sys_fcntl{,64} for this patch, looks
> good in that respect.
> 
> > +error:
> > +	free_page((unsigned long) buf);
> > +	return size;
> > +}
> > +EXPORT_SYMBOL(fcntl_getpath);
> > +
> 
> Procfs cannot be a module, and there are no other users of this function,
> so why do you export it?

Springtime export fever? :)
I dropped the export. I'll repost as soon as I've five minutes to reboot 
my box (to test the thing a little).



- Davide

Re: [patch] F_GETPATH for linux

[Michael Kerrisk]

On 4/14/08, Davide Libenzi <davidel@xmailserver.org> wrote:
> On Mon, 14 Apr 2008, Arnd Bergmann wrote:
>
>  > On Monday 14 April 2008, Davide Libenzi wrote:
>  > > I was working on some bsd/linux compatibility code, and I noticed that we
>  > > do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is
>  > > what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
>  > > Here it is, building but untested ...
>  >
>  > I reviewed the code path for compat_sys_fcntl{,64} for this patch, looks
>  > good in that respect.
>  >
>  > > +error:
>  > > +   free_page((unsigned long) buf);
>  > > +   return size;
>  > > +}
>  > > +EXPORT_SYMBOL(fcntl_getpath);
>  > > +
>  >
>  > Procfs cannot be a module, and there are no other users of this function,
>  > so why do you export it?
>
>
> Springtime export fever? :)
>  I dropped the export. I'll repost as soon as I've five minutes to reboot
>  my box (to test the thing a little).

Davide,

Please CC me on all kernel-userland API changes, so that I can watch
for changes that may be needed for man-pages.

Cheers,

Michael

-- 
I'll likely only see replies if they are CCed to mtk.manpages at gmail dot com

Re: [patch] F_GETPATH for linux

[Davide Libenzi]

On Mon, 14 Apr 2008, Michael Kerrisk wrote:

> On 4/14/08, Davide Libenzi <davidel@xmailserver.org> wrote:
> > On Mon, 14 Apr 2008, Arnd Bergmann wrote:
> >
> >  > On Monday 14 April 2008, Davide Libenzi wrote:
> >  > > I was working on some bsd/linux compatibility code, and I noticed that we
> >  > > do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is
> >  > > what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
> >  > > Here it is, building but untested ...
> >  >
> >  > I reviewed the code path for compat_sys_fcntl{,64} for this patch, looks
> >  > good in that respect.
> >  >
> >  > > +error:
> >  > > +   free_page((unsigned long) buf);
> >  > > +   return size;
> >  > > +}
> >  > > +EXPORT_SYMBOL(fcntl_getpath);
> >  > > +
> >  >
> >  > Procfs cannot be a module, and there are no other users of this function,
> >  > so why do you export it?
> >
> >
> > Springtime export fever? :)
> >  I dropped the export. I'll repost as soon as I've five minutes to reboot
> >  my box (to test the thing a little).
> 
> Davide,
> 
> Please CC me on all kernel-userland API changes, so that I can watch
> for changes that may be needed for man-pages.

Michael, I will.



- Davide

Re: [patch] F_GETPATH for linux

[Christoph Hellwig]

On Sun, Apr 13, 2008 at 03:00:53PM -0700, Davide Libenzi wrote:
> I was working on some bsd/linux compatibility code, and I noticed that we 
> do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
> what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
> Here it is, building but untested ...

Adding a duplicated API like this is rather pointless.  Especially as
the readlink is easier and makes more sense.

Re: [patch] F_GETPATH for linux

[H. Peter Anvin]

Christoph Hellwig wrote:
> On Sun, Apr 13, 2008 at 03:00:53PM -0700, Davide Libenzi wrote:
>> I was working on some bsd/linux compatibility code, and I noticed that we 
>> do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
>> what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
>> Here it is, building but untested ...
> 
> Adding a duplicated API like this is rather pointless.  Especially as
> the readlink is easier and makes more sense.

Yes; also, using /proc/self/fd/... makes it a little easier.

	-hpa

Re: [patch] F_GETPATH for linux

[Linus Torvalds]

On Mon, 14 Apr 2008, Christoph Hellwig wrote:
> 
> Adding a duplicated API like this is rather pointless.  Especially as
> the readlink is easier and makes more sense.

Well, I wouldn't say "makes more sense". It may be more traditional (for 
Linux), but it has its problems, notably performance. One use of F_GETPATH 
is for server code, to check that a previous open() didn't follow symlinks 
to outside of some specified area.

Not that I like F_GETPATH much either, but portability is a strong enough 
argument that coming up with some *other* way is probably not great.

			Linus

Re: [patch] F_GETPATH for linux

[Davide Libenzi]

On Mon, 14 Apr 2008, Christoph Hellwig wrote:

> On Sun, Apr 13, 2008 at 03:00:53PM -0700, Davide Libenzi wrote:
> > I was working on some bsd/linux compatibility code, and I noticed that we 
> > do not have F_GETPATH. I know we can readlink /proc/PID/fd/FD (that is 
> > what I'm doing now, under #ifdef), but I gave a patch a shot anyway.
> > Here it is, building but untested ...
> 
> Adding a duplicated API like this is rather pointless.  Especially as
> the readlink is easier and makes more sense.

As I said, we can do it in the other way. I struck this one when trying to 
work out (read port) some BSD code to Linux. IMO, given that the patch 
does virtually only add an F_GETPATH define in fcntl.h, and a case in the 
fcntl switch, it may be worth considering.



- Davide