2002-03-08 09:21:32

by Paul Menage

[permalink] [raw]
Subject: [PATCH] More user-space path handling cleanups


This patch (against 2.5.6-pre3) replaces a few instances of the
getname()/path_lookup() combination with __user_walk(), in the same vein
as some of the changes accompanying the path_lookup() patch that was
included in 2.5.6-pre3.

namei.c | 60 +++++++++++-------------------------------------------------
1 files changed, 11 insertions(+), 49 deletions(-)


diff -aur linux-2.5.6-pre3/fs/namei.c linux-2.5.6-pre3.new/fs/namei.c
--- linux-2.5.6-pre3/fs/namei.c Thu Mar 7 17:12:11 2002
+++ linux-2.5.6-pre3.new/fs/namei.c Thu Mar 7 17:24:51 2002
@@ -1259,17 +1259,12 @@
asmlinkage long sys_mknod(const char * filename, int mode, dev_t dev)
{
int error = 0;
- char * tmp;
struct dentry * dentry;
struct nameidata nd;

if (S_ISDIR(mode))
return -EPERM;
- tmp = getname(filename);
- if (IS_ERR(tmp))
- return PTR_ERR(tmp);
-
- error = path_lookup(tmp, LOOKUP_PARENT, &nd);
+ error = __user_walk(filename, LOOKUP_PARENT, &nd);
if (error)
goto out;
dentry = lookup_create(&nd, 0);
@@ -1295,7 +1290,6 @@
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
out:
- putname(tmp);

return error;
}
@@ -1321,17 +1315,11 @@
asmlinkage long sys_mkdir(const char * pathname, int mode)
{
int error = 0;
- char * tmp;
-
- tmp = getname(pathname);
- error = PTR_ERR(tmp);
- if (!IS_ERR(tmp)) {
+ struct nameidata nd;
+ error = __user_walk(pathname, LOOKUP_PARENT, &nd);
+ if (!error) {
struct dentry *dentry;
- struct nameidata nd;

- error = path_lookup(tmp, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
dentry = lookup_create(&nd, 1);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
@@ -1341,8 +1329,6 @@
}
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
-out:
- putname(tmp);
}

return error;
@@ -1412,15 +1398,10 @@
asmlinkage long sys_rmdir(const char * pathname)
{
int error = 0;
- char * name;
struct dentry *dentry;
struct nameidata nd;

- name = getname(pathname);
- if(IS_ERR(name))
- return PTR_ERR(name);
-
- error = path_lookup(name, LOOKUP_PARENT, &nd);
+ error = __user_walk(pathname, LOOKUP_PARENT, &nd);
if (error)
goto exit;

@@ -1446,7 +1427,6 @@
exit1:
path_release(&nd);
exit:
- putname(name);
return error;
}

@@ -1483,15 +1463,10 @@
asmlinkage long sys_unlink(const char * pathname)
{
int error = 0;
- char * name;
struct dentry *dentry;
struct nameidata nd;

- name = getname(pathname);
- if(IS_ERR(name))
- return PTR_ERR(name);
-
- error = path_lookup(name, LOOKUP_PARENT, &nd);
+ error = __user_walk(pathname, LOOKUP_PARENT, &nd);
if (error)
goto exit;
error = -EISDIR;
@@ -1512,7 +1487,6 @@
exit1:
path_release(&nd);
exit:
- putname(name);

return error;

@@ -1543,20 +1517,16 @@
{
int error = 0;
char * from;
- char * to;
+ struct nameidata nd;

from = getname(oldname);
if(IS_ERR(from))
return PTR_ERR(from);
- to = getname(newname);
- error = PTR_ERR(to);
- if (!IS_ERR(to)) {
+
+ error = __user_walk(newname, LOOKUP_PARENT, &nd);
+ if(!error) {
struct dentry *dentry;
- struct nameidata nd;

- error = path_lookup(to, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
dentry = lookup_create(&nd, 0);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
@@ -1565,8 +1535,6 @@
}
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
-out:
- putname(to);
}
putname(from);
return error;
@@ -1620,16 +1588,11 @@
struct dentry *new_dentry;
struct nameidata nd, old_nd;
int error;
- char * to;
-
- to = getname(newname);
- if (IS_ERR(to))
- return PTR_ERR(to);

error = __user_walk(oldname, 0, &old_nd);
if (error)
goto exit;
- error = path_lookup(to, LOOKUP_PARENT, &nd);
+ error = __user_walk(newname, LOOKUP_PARENT, &nd);
if (error)
goto out;
error = -EXDEV;
@@ -1647,7 +1610,6 @@
out:
path_release(&old_nd);
exit:
- putname(to);

return error;
}



2002-03-08 09:26:03

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] More user-space path handling cleanups



On Fri, 8 Mar 2002, Paul Menage wrote:

>
> This patch (against 2.5.6-pre3) replaces a few instances of the
> getname()/path_lookup() combination with __user_walk(), in the same vein
> as some of the changes accompanying the path_lookup() patch that was
> included in 2.5.6-pre3.

Broken. __user_walk() frees the temporary name it had created, leaving
nd->last.name pointing nowhere (BTW, that's how I fucked up in the first
version of patch that went into -pre3).

2002-03-08 10:48:49

by Paul Menage

[permalink] [raw]
Subject: Re: [PATCH] More user-space path handling cleanups

>
>Broken. __user_walk() frees the temporary name it had created, leaving
>nd->last.name pointing nowhere (BTW, that's how I fucked up in the first
>version of patch that went into -pre3).
>

Oops. I guess I was wondering why you'd missed some of the getname()/
path_lookup() instances. How about this one instead? It adds a function
getname_walk() that returns the temporary buffer if the lookup is
successful, or else frees the buffer and returns the error. Caller is
responsible for freeing the buffer if it is returned. (Could maybe use a
better name than getname_walk().)

namei.c | 66 +++++++++++++++++++++++++---------------------------------------
1 files changed, 26 insertions(+), 40 deletions(-)


--- linux-2.5.6-pre3/fs/namei.c Thu Mar 7 17:12:11 2002
+++ linux-2.5.6-pre3.new/fs/namei.c Fri Mar 8 02:29:47 2002
@@ -817,16 +817,27 @@
* that namei follows links, while lnamei does not.
* SMP-safe
*/
-int __user_walk(const char *name, unsigned flags, struct nameidata *nd)
+static inline char *getname_walk(const char *name, unsigned flags, struct nameidata *nd)
{
char *tmp = getname(name);
- int err = PTR_ERR(tmp);

if (!IS_ERR(tmp)) {
- err = path_lookup(tmp, flags, nd);
- putname(tmp);
+ int err = path_lookup(tmp, flags, nd);
+ if(err) {
+ putname(tmp);
+ tmp = ERR_PTR(err);
+ }
}
- return err;
+ return tmp;
+}
+
+int __user_walk(const char *name, unsigned flags, struct nameidata *nd)
+{
+ char *tmp = getname_walk(name, flags, nd);
+ if(IS_ERR(tmp))
+ return PTR_ERR(tmp);
+ putname(tmp);
+ return 0;
}

/*
@@ -1265,13 +1276,10 @@

if (S_ISDIR(mode))
return -EPERM;
- tmp = getname(filename);
+ tmp = getname_walk(filename, LOOKUP_PARENT, &nd);
if (IS_ERR(tmp))
return PTR_ERR(tmp);

- error = path_lookup(tmp, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
dentry = lookup_create(&nd, 0);
error = PTR_ERR(dentry);

@@ -1294,7 +1302,6 @@
}
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
-out:
putname(tmp);

return error;
@@ -1322,16 +1329,13 @@
{
int error = 0;
char * tmp;
+ struct nameidata nd;

- tmp = getname(pathname);
+ tmp = getname_walk(pathname, LOOKUP_PARENT, &nd);
error = PTR_ERR(tmp);
if (!IS_ERR(tmp)) {
struct dentry *dentry;
- struct nameidata nd;

- error = path_lookup(tmp, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
dentry = lookup_create(&nd, 1);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
@@ -1341,7 +1345,6 @@
}
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
-out:
putname(tmp);
}

@@ -1416,14 +1419,10 @@
struct dentry *dentry;
struct nameidata nd;

- name = getname(pathname);
+ name = getname_walk(pathname, LOOKUP_PARENT, &nd);
if(IS_ERR(name))
return PTR_ERR(name);

- error = path_lookup(name, LOOKUP_PARENT, &nd);
- if (error)
- goto exit;
-
switch(nd.last_type) {
case LAST_DOTDOT:
error = -ENOTEMPTY;
@@ -1445,7 +1444,6 @@
up(&nd.dentry->d_inode->i_sem);
exit1:
path_release(&nd);
-exit:
putname(name);
return error;
}
@@ -1487,13 +1485,10 @@
struct dentry *dentry;
struct nameidata nd;

- name = getname(pathname);
+ name = getname_walk(pathname, LOOKUP_PARENT, &nd);
if(IS_ERR(name))
return PTR_ERR(name);

- error = path_lookup(name, LOOKUP_PARENT, &nd);
- if (error)
- goto exit;
error = -EISDIR;
if (nd.last_type != LAST_NORM)
goto exit1;
@@ -1511,7 +1506,6 @@
up(&nd.dentry->d_inode->i_sem);
exit1:
path_release(&nd);
-exit:
putname(name);

return error;
@@ -1544,19 +1538,16 @@
int error = 0;
char * from;
char * to;
+ struct nameidata nd;

from = getname(oldname);
if(IS_ERR(from))
return PTR_ERR(from);
- to = getname(newname);
+ to = getname_walk(newname, LOOKUP_PARENT, &nd);
error = PTR_ERR(to);
if (!IS_ERR(to)) {
struct dentry *dentry;
- struct nameidata nd;

- error = path_lookup(to, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
dentry = lookup_create(&nd, 0);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
@@ -1565,7 +1556,6 @@
}
up(&nd.dentry->d_inode->i_sem);
path_release(&nd);
-out:
putname(to);
}
putname(from);
@@ -1622,19 +1612,16 @@
int error;
char * to;

- to = getname(newname);
+ to = getname_walk(newname, LOOKUP_PARENT, &nd);
if (IS_ERR(to))
return PTR_ERR(to);

error = __user_walk(oldname, 0, &old_nd);
if (error)
goto exit;
- error = path_lookup(to, LOOKUP_PARENT, &nd);
- if (error)
- goto out;
error = -EXDEV;
if (old_nd.mnt != nd.mnt)
- goto out_release;
+ goto out;
new_dentry = lookup_create(&nd, 0);
error = PTR_ERR(new_dentry);
if (!IS_ERR(new_dentry)) {
@@ -1642,11 +1629,10 @@
dput(new_dentry);
}
up(&nd.dentry->d_inode->i_sem);
-out_release:
- path_release(&nd);
out:
path_release(&old_nd);
exit:
+ path_release(&nd);
putname(to);

return error;