| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4B7FD479.5020807@canonical.com>
Date: Sat, 20 Feb 2010 04:24:25 -0800
From: John Johansen <john.johansen@...onical.com>
To: Al Viro <viro@...IV.linux.org.uk>
CC: linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Subject: Re: [PATCH 01/12] Miscellaneous functions and defines needed by AppArmor,
including the base path resolution routines.
Al Viro wrote:
> On Fri, Feb 19, 2010 at 01:36:17AM -0800, john.johansen@...onical.com wrote:
>
>> + spin_lock(&dcache_lock);
>> + /* There is a race window between path lookup here and the
>> + * need to strip the " (deleted) string that __d_path applies
>> + * Detect the race and relookup the path
>> + *
>> + * The stripping of (deleted) is a hack that could be removed
>> + * with an updated __d_path
>
> Yes, it could. Where's the patch doing just that? Or discussion of
> desired interface, at lease...
>
Glad you asked I was going to include it with a couple other patches to
__d_path. Basically trying to separate proposed changes to __d_path
from AppArmor, as most people who will be looking at the __d_path code
would rather not have the whole AppArmor patchset dropped on them.
The attached patch is just a first pass and a starting point for
discussion. I only modified TOMOYO to match current behavior of the
kernel and will leave it to Tetsuo to modify TOMOYO to make use
of the flag.
---
Make __d_path unambiguous for deleted files
__d_path currently appends the string " (deleted)" to deleted entries, but
this results in an ambiguous path. This is problematic for TOMOYO and
AppArmor as they use __d_path to retrieve path names for file objects.
This patch matches the appending the " (deleted)" string optional,
removing the need for AppArmor and TOMOYO to remove the string after
the fact.
Signed-off-by: John Johansen <john.johansen@...onical.com>
diff --git a/fs/dcache.c b/fs/dcache.c
index df49666..44c2afc 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1890,9 +1890,10 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
* @root: root vfsmnt/dentry (may be modified by this function)
* @buffer: buffer to return value in
* @buflen: buffer length
- *
+ * @mark_deleted: for deleted entries determine whether to append " (deleted)"
* Convert a dentry into an ASCII path name. If the entry has been deleted
- * the string " (deleted)" is appended. Note that this is ambiguous.
+ * and @mark_deleted is true then the string " (deleted)" is appended.
+ * Note that this is ambiguous.
*
* Returns a pointer into the buffer or an error code if the
* path was too long.
@@ -1903,7 +1904,7 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
* root is changed (without modifying refcounts).
*/
char *__d_path(const struct path *path, struct path *root,
- char *buffer, int buflen)
+ char *buffer, int buflen, bool mark_deleted)
{
struct dentry *dentry = path->dentry;
struct vfsmount *vfsmnt = path->mnt;
@@ -1912,7 +1913,7 @@ char *__d_path(const struct path *path, struct path *root,
spin_lock(&vfsmount_lock);
prepend(&end, &buflen, "\0", 1);
- if (d_unlinked(dentry) &&
+ if (d_unlinked(dentry) && mark_deleted &&
(prepend(&end, &buflen, " (deleted)", 10) != 0))
goto Elong;
@@ -2019,7 +2020,7 @@ char *d_path(const struct path *path, char *buf, int buflen)
read_unlock(¤t->fs->lock);
spin_lock(&dcache_lock);
tmp = root;
- res = __d_path(path, &tmp, buf, buflen);
+ res = __d_path(path, &tmp, buf, buflen, DPATH_MARK_DELETED);
spin_unlock(&dcache_lock);
path_put(&root);
return res;
@@ -2124,7 +2125,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
struct path tmp = root;
char * cwd;
- cwd = __d_path(&pwd, &tmp, page, PAGE_SIZE);
+ cwd = __d_path(&pwd, &tmp, page, PAGE_SIZE, DPATH_MARK_DELETED);
spin_unlock(&dcache_lock);
error = PTR_ERR(cwd);
diff --git a/fs/seq_file.c b/fs/seq_file.c
index eae7d9d..f046bd3 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -463,7 +463,7 @@ int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
char *p;
spin_lock(&dcache_lock);
- p = __d_path(path, root, buf, size);
+ p = __d_path(path, root, buf, size, DPATH_MARK_DELETED);
spin_unlock(&dcache_lock);
res = PTR_ERR(p);
if (!IS_ERR(p)) {
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 30b93b2..021f515 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -309,9 +309,12 @@ extern int d_validate(struct dentry *, struct dentry *);
/*
* helper function for dentry_operations.d_dname() members
*/
+#define DPATH_MARK_DELETED 1
+
extern char *dynamic_dname(struct dentry *, char *, int, const char *, ...);
-extern char *__d_path(const struct path *path, struct path *root, char *, int);
+extern char *__d_path(const struct path *path, struct path *root, char *, int,
+ bool);
extern char *d_path(const struct path *, char *, int);
extern char *dentry_path(struct dentry *, char *, int);
diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
index 18369d4..0f4d4e2 100644
--- a/security/tomoyo/realpath.c
+++ b/security/tomoyo/realpath.c
@@ -106,7 +106,8 @@ int tomoyo_realpath_from_path2(struct path *path, char *newname,
spin_unlock(&vfsmount_lock);
spin_lock(&dcache_lock);
tmp = ns_root;
- sp = __d_path(path, &tmp, newname, newname_len);
+ sp = __d_path(path, &tmp, newname, newname_len,
+ DPATH_MARK_DELETED);
spin_unlock(&dcache_lock);
path_put(&root);
path_put(&ns_root);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists