From: Miklos Szeredi <[email protected]>
__d_path() has 4 callers:
d_path()
sys_getcwd()
seq_path_root()
tomoyo_realpath_from_path2()
Of these the only one which needs the " (deleted)" ending is d_path().
sys_getcwd() checks for existence before calling __d_path().
seq_path_root() is used to show the mountpoint path in
/proc/PID/mountinfo, which is always a positive.
And tomoyo doesn't want the deleted ending.
Create a helper "path_with_deleted()" as subsequent patches will need
this in multiple places.
Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/dcache.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-07-06 18:08:16.000000000 +0200
+++ linux-2.6/fs/dcache.c 2010-07-06 18:08:19.000000000 +0200
@@ -1977,8 +1977,7 @@ global_root:
* @buffer: buffer to return value in
* @buflen: buffer length
*
- * Convert a dentry into an ASCII path name. If the entry has been deleted
- * the string " (deleted)" is appended. Note that this is ambiguous.
+ * Convert a dentry into an ASCII path name.
*
* Returns a pointer into the buffer or an error code if the
* path was too long.
@@ -1995,12 +1994,6 @@ char *__d_path(const struct path *path,
int error;
prepend(&res, &buflen, "\0", 1);
- if (d_unlinked(path->dentry)) {
- error = prepend(&res, &buflen, " (deleted)", 10);
- if (error)
- return ERR_PTR(error);
- }
-
error = prepend_path(path, root, &res, &buflen);
if (error)
return ERR_PTR(error);
@@ -2008,6 +2001,22 @@ char *__d_path(const struct path *path,
return res;
}
+/*
+ * same as __d_path but appends "(deleted)" for unlinked files.
+ */
+static int path_with_deleted(const struct path *path, struct path *root,
+ char **buf, int *buflen)
+{
+ prepend(buf, buflen, "\0", 1);
+ if (d_unlinked(path->dentry)) {
+ int error = prepend(buf, buflen, " (deleted)", 10);
+ if (error)
+ return error;
+ }
+
+ return prepend_path(path, root, buf, buflen);
+}
+
/**
* d_path - return the path of a dentry
* @path: path to report
@@ -2026,9 +2035,10 @@ char *__d_path(const struct path *path,
*/
char *d_path(const struct path *path, char *buf, int buflen)
{
- char *res;
+ char *res = buf + buflen;
struct path root;
struct path tmp;
+ int error;
/*
* We have various synthetic filesystems that never get mounted. On
@@ -2043,7 +2053,9 @@ char *d_path(const struct path *path, ch
get_fs_root(current->fs, &root);
spin_lock(&dcache_lock);
tmp = root;
- res = __d_path(path, &tmp, buf, buflen);
+ error = path_with_deleted(path, &tmp, &res, &buflen);
+ if (error)
+ res = ERR_PTR(error);
spin_unlock(&dcache_lock);
path_put(&root);
return res;
--
On Mon, 02 Aug 2010 13:20:01 +0200, Miklos Szeredi said:
> Index: linux-2.6/fs/dcache.c
> ===================================================================
> --- linux-2.6.orig/fs/dcache.c 2010-07-06 18:08:16.000000000 +0200
> +++ linux-2.6/fs/dcache.c 2010-07-06 18:08:19.000000000 +0200
> @@ -1977,8 +1977,7 @@ global_root:
> * @buffer: buffer to return value in
> * @buflen: buffer length
> *
> - * Convert a dentry into an ASCII path name. If the entry has been deleted
> - * the string " (deleted)" is appended. Note that this is ambiguous.
> + * Convert a dentry into an ASCII path name.
> *
> * Returns a pointer into the buffer or an error code if the
> * path was too long.
I'd prefer the comment about it being ambiguous remain. I'm waiting to see how
long it takes for somebody to create a security hole by creating a file called
'/etc/some/thing/important (deleted)' and having some software Do The Wrong
Thing instead to /etc/some/thing/important.
On Mon, Aug 2, 2010 at 3:00 PM, <[email protected]> wrote:
> On Mon, 02 Aug 2010 13:20:01 +0200, Miklos Szeredi said:
>
>> Index: linux-2.6/fs/dcache.c
>> ===================================================================
>> --- linux-2.6.orig/fs/dcache.c ? ? ? ?2010-07-06 18:08:16.000000000 +0200
>> +++ linux-2.6/fs/dcache.c ? ? 2010-07-06 18:08:19.000000000 +0200
>> @@ -1977,8 +1977,7 @@ global_root:
>> ? * @buffer: buffer to return value in
>> ? * @buflen: buffer length
>> ? *
>> - * Convert a dentry into an ASCII path name. If the entry has been deleted
>> - * the string " (deleted)" is appended. Note that this is ambiguous.
>> + * Convert a dentry into an ASCII path name.
>> ? *
>> ? * Returns a pointer into the buffer or an error code if the
>> ? * path was too long.
>
> I'd prefer the comment about it being ambiguous remain. ?I'm waiting to see how
> long it takes for somebody to create a security hole by creating a file called
> '/etc/some/thing/important (deleted)' and having some software Do The Wrong
> Thing instead to /etc/some/thing/important.
>
In order to close this kind of hole why not creating a deleted
directory on /proc and redirect symbolic link to this directory.
And do the same for unreachable. If we use the good permission it will
work from a backaward compatibily point of view
bastien
On Mon, 02 Aug 2010, [email protected] wrote:
> On Mon, 02 Aug 2010 13:20:01 +0200, Miklos Szeredi said:
>
> > Index: linux-2.6/fs/dcache.c
> > ===================================================================
> > --- linux-2.6.orig/fs/dcache.c 2010-07-06 18:08:16.000000000 +0200
> > +++ linux-2.6/fs/dcache.c 2010-07-06 18:08:19.000000000 +0200
> > @@ -1977,8 +1977,7 @@ global_root:
> > * @buffer: buffer to return value in
> > * @buflen: buffer length
> > *
> > - * Convert a dentry into an ASCII path name. If the entry has been deleted
> > - * the string " (deleted)" is appended. Note that this is ambiguous.
> > + * Convert a dentry into an ASCII path name.
> > *
> > * Returns a pointer into the buffer or an error code if the
> > * path was too long.
>
> I'd prefer the comment about it being ambiguous remain. I'm waiting
> to see how long it takes for somebody to create a security hole by
> creating a file called '/etc/some/thing/important (deleted)' and
> having some software Do The Wrong Thing instead to
> /etc/some/thing/important.
The same comment is left intact on top of d_path().
It is removed from __d_path() because this function no longer appends
"(deleted)".
Thanks,
Miklos
On Mon, 2 Aug 2010, Bastien ROUCARIES wrote:
> On Mon, Aug 2, 2010 at 3:00 PM, <[email protected]> wrote:
> > On Mon, 02 Aug 2010 13:20:01 +0200, Miklos Szeredi said:
> >
> >> Index: linux-2.6/fs/dcache.c
> >> ===================================================================
> >> --- linux-2.6.orig/fs/dcache.c 2010-07-06 18:08:16.000000000 +0200
> >> +++ linux-2.6/fs/dcache.c 2010-07-06 18:08:19.000000000 +0200
> >> @@ -1977,8 +1977,7 @@ global_root:
> >> * @buffer: buffer to return value in
> >> * @buflen: buffer length
> >> *
> >> - * Convert a dentry into an ASCII path name. If the entry has been deleted
> >> - * the string " (deleted)" is appended. Note that this is ambiguous.
> >> + * Convert a dentry into an ASCII path name.
> >> *
> >> * Returns a pointer into the buffer or an error code if the
> >> * path was too long.
> >
> > I'd prefer the comment about it being ambiguous remain. I'm waiting to see how
> > long it takes for somebody to create a security hole by creating a file called
> > '/etc/some/thing/important (deleted)' and having some software Do The Wrong
> > Thing instead to /etc/some/thing/important.
> >
>
> In order to close this kind of hole why not creating a deleted
> directory on /proc and redirect symbolic link to this directory.
> And do the same for unreachable. If we use the good permission it will
> work from a backaward compatibily point of view
Unfortunately the "(deleted)" thing is part of the userspace ABI and
can't be changed.
The only thing we can do is make sure new interfaces use a saner
convention.
Thanks,
Miklos
On Mon, 02 Aug 2010 15:35:20 +0200, Miklos Szeredi said:
> > I'd prefer the comment about it being ambiguous remain. I'm waiting
> > to see how long it takes for somebody to create a security hole by
> > creating a file called '/etc/some/thing/important (deleted)' and
> > having some software Do The Wrong Thing instead to
> > /etc/some/thing/important.
>
> The same comment is left intact on top of d_path().
>
> It is removed from __d_path() because this function no longer appends
> "(deleted)".
Oh, OK. Obviously -ENOCAFFEINE on my part, I thought you were nuking the
d_path() copy. ;)