Re: [git pull] apparmor fix for __d_path() misuse

[Linus Torvalds]

On Tue, Dec 6, 2011 at 3:12 PM, John Johansen
<john.johansen@xxxxxxxxxxxxx> wrote:
>
> What we want to know is if we missed the supplied root, so that we don't
> mediate off of that path.  And it would be nice to get a partial path for
> the purposes of logging, so that there is some guidance on how to update
> policy.

How about this change:
 - don't change 'root' (and mark it const)
 - if we hit the expected root, we're all happy and do what we do now
 - if we hit some *unexpected* root (the "global root") add a '?' or
something at the head of the path.

End result: callers like getcwd() can trivially replace their current
"path_equal(&tmp,&root)" (or whatever they do) with just checking the
first character of the end result. A good path always starts with '/'.

Something kind of like this (this does *not* change apparmor or tomoyo
- I didn't even look at those uses).

Comments?

               Linus
 fs/dcache.c              |   12 +++++-------
 fs/seq_file.c            |    6 ++----
 include/linux/dcache.h   |    2 +-
 include/linux/seq_file.h |    4 ++--
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 10ba92def3f6..93e063f713f5 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2448,7 +2448,7 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
  * If path is not reachable from the supplied root, then the value of
  * root is changed (without modifying refcounts).
  */
-static int prepend_path(const struct path *path, struct path *root,
+static int prepend_path(const struct path *path, const struct path *root,
 			char **buffer, int *buflen)
 {
 	struct dentry *dentry = path->dentry;
@@ -2500,8 +2500,7 @@ global_root:
 		WARN(1, "Root dentry has weird name <%.*s>\n",
 		     (int) dentry->d_name.len, dentry->d_name.name);
 	}
-	root->mnt = vfsmnt;
-	root->dentry = dentry;
+	error = prepend(buffer, buflen, "?", 1);
 	goto out;
 }
 
@@ -2522,7 +2521,7 @@ global_root:
  * If path is not reachable from the supplied root, then the value of
  * root is changed (without modifying refcounts).
  */
-char *__d_path(const struct path *path, struct path *root,
+char *__d_path(const struct path *path, const struct path *root,
 	       char *buf, int buflen)
 {
 	char *res = buf + buflen;
@@ -2758,19 +2757,18 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
 	write_seqlock(&rename_lock);
 	if (!d_unlinked(pwd.dentry)) {
 		unsigned long len;
-		struct path tmp = root;
 		char *cwd = page + PAGE_SIZE;
 		int buflen = PAGE_SIZE;
 
 		prepend(&cwd, &buflen, "\0", 1);
-		error = prepend_path(&pwd, &tmp, &cwd, &buflen);
+		error = prepend_path(&pwd, &root, &cwd, &buflen);
 		write_sequnlock(&rename_lock);
 
 		if (error)
 			goto out;
 
 		/* Unreachable from current root */
-		if (!path_equal(&tmp, &root)) {
+		if (cwd[0] != '/') {
 			error = prepend_unreachable(&cwd, &buflen);
 			if (error)
 				goto out;
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 05d6b0e78c95..d5a128746974 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -427,7 +427,7 @@ EXPORT_SYMBOL(mangle_path);
  * return the absolute path of 'path', as represented by the
  * dentry / mnt pair in the path parameter.
  */
-int seq_path(struct seq_file *m, struct path *path, char *esc)
+int seq_path(struct seq_file *m, const struct path *path, char *esc)
 {
 	char *buf;
 	size_t size = seq_get_buf(m, &buf);
@@ -449,10 +449,8 @@ EXPORT_SYMBOL(seq_path);
 
 /*
  * Same as seq_path, but relative to supplied root.
- *
- * root may be changed, see __d_path().
  */
-int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
+int seq_path_root(struct seq_file *m, const struct path *path, const struct path *root,
 		  char *esc)
 {
 	char *buf;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 4df926199369..581feaba8260 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -339,7 +339,7 @@ extern int d_validate(struct dentry *, struct dentry *);
  */
 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, const struct path *root, char *, int);
 extern char *d_path(const struct path *, char *, int);
 extern char *d_path_with_unreachable(const struct path *, char *, int);
 extern char *dentry_path_raw(struct dentry *, char *, int);
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 0b69a4684216..0b0a015c553a 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -86,9 +86,9 @@ int seq_write(struct seq_file *seq, const void *data, size_t len);
 
 __printf(2, 3) int seq_printf(struct seq_file *, const char *, ...);
 
-int seq_path(struct seq_file *, struct path *, char *);
+int seq_path(struct seq_file *, const struct path *, char *);
 int seq_dentry(struct seq_file *, struct dentry *, char *);
-int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
+int seq_path_root(struct seq_file *m, const struct path *path, const struct path *root,
 		  char *esc);
 int seq_bitmap(struct seq_file *m, const unsigned long *bits,
 				   unsigned int nr_bits);