From 283d386f981c57479d93e392416be9a704de258b Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 9 Dec 2011 13:50:19 -0800 Subject: [PATCH] 3.1 patches added patches: fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch tomoyo-fix-pathname-handling-of-disconnected-paths.patch --- ...y-freed-dentry-sanitize-__d_path-api.patch | 447 ++++++++++++++++++ queue-3.1/series | 2 + ...hname-handling-of-disconnected-paths.patch | 45 ++ 3 files changed, 494 insertions(+) create mode 100644 queue-3.1/fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch create mode 100644 queue-3.1/tomoyo-fix-pathname-handling-of-disconnected-paths.patch diff --git a/queue-3.1/fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch b/queue-3.1/fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch new file mode 100644 index 00000000000..421193a2cd4 --- /dev/null +++ b/queue-3.1/fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch @@ -0,0 +1,447 @@ +From 02125a826459a6ad142f8d91c5b6357562f96615 Mon Sep 17 00:00:00 2001 +From: Al Viro +Date: Mon, 5 Dec 2011 08:43:34 -0500 +Subject: fix apparmor dereferencing potentially freed dentry, sanitize __d_path() API + +From: Al Viro + +commit 02125a826459a6ad142f8d91c5b6357562f96615 upstream. + +__d_path() API is asking for trouble and in case of apparmor d_namespace_path() +getting just that. The root cause is that when __d_path() misses the root +it had been told to look for, it stores the location of the most remote ancestor +in *root. Without grabbing references. Sure, at the moment of call it had +been pinned down by what we have in *path. And if we raced with umount -l, we +could have very well stopped at vfsmount/dentry that got freed as soon as +prepend_path() dropped vfsmount_lock. + +It is safe to compare these pointers with pre-existing (and known to be still +alive) vfsmount and dentry, as long as all we are asking is "is it the same +address?". Dereferencing is not safe and apparmor ended up stepping into +that. d_namespace_path() really wants to examine the place where we stopped, +even if it's not connected to our namespace. As the result, it looked +at ->d_sb->s_magic of a dentry that might've been already freed by that point. +All other callers had been careful enough to avoid that, but it's really +a bad interface - it invites that kind of trouble. + +The fix is fairly straightforward, even though it's bigger than I'd like: + * prepend_path() root argument becomes const. + * __d_path() is never called with NULL/NULL root. It was a kludge +to start with. Instead, we have an explicit function - d_absolute_root(). +Same as __d_path(), except that it doesn't get root passed and stops where +it stops. apparmor and tomoyo are using it. + * __d_path() returns NULL on path outside of root. The main +caller is show_mountinfo() and that's precisely what we pass root for - to +skip those outside chroot jail. Those who don't want that can (and do) +use d_path(). + * __d_path() root argument becomes const. Everyone agrees, I hope. + * apparmor does *NOT* try to use __d_path() or any of its variants +when it sees that path->mnt is an internal vfsmount. In that case it's +definitely not mounted anywhere and dentry_path() is exactly what we want +there. Handling of sysctl()-triggered weirdness is moved to that place. + * if apparmor is asked to do pathname relative to chroot jail +and __d_path() tells it we it's not in that jail, the sucker just calls +d_absolute_path() instead. That's the other remaining caller of __d_path(), +BTW. + * seq_path_root() does _NOT_ return -ENAMETOOLONG (it's stupid anyway - +the normal seq_file logics will take care of growing the buffer and redoing +the call of ->show() just fine). However, if it gets path not reachable +from root, it returns SEQ_SKIP. The only caller adjusted (i.e. stopped +ignoring the return value as it used to do). + +Reviewed-by: John Johansen +ACKed-by: John Johansen +Signed-off-by: Al Viro +Signed-off-by: Greg Kroah-Hartman + +--- + fs/dcache.c | 71 +++++++++++++++++++++++++++------------------ + fs/namespace.c | 20 ++++++------ + fs/seq_file.c | 6 +-- + include/linux/dcache.h | 3 + + include/linux/fs.h | 1 + security/apparmor/path.c | 65 ++++++++++++++++++++++++----------------- + security/tomoyo/realpath.c | 3 - + 7 files changed, 100 insertions(+), 69 deletions(-) + +--- a/fs/dcache.c ++++ b/fs/dcache.c +@@ -2398,16 +2398,14 @@ static int prepend_name(char **buffer, i + /** + * prepend_path - Prepend path string to a buffer + * @path: the dentry/vfsmount to report +- * @root: root vfsmnt/dentry (may be modified by this function) ++ * @root: root vfsmnt/dentry + * @buffer: pointer to the end of the buffer + * @buflen: pointer to buffer length + * + * Caller holds the rename_lock. +- * +- * 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; +@@ -2442,10 +2440,10 @@ static int prepend_path(const struct pat + dentry = parent; + } + +-out: + if (!error && !slash) + error = prepend(buffer, buflen, "/", 1); + ++out: + br_read_unlock(vfsmount_lock); + return error; + +@@ -2459,15 +2457,17 @@ 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; ++ if (!slash) ++ error = prepend(buffer, buflen, "/", 1); ++ if (!error) ++ error = vfsmnt->mnt_ns ? 1 : 2; + goto out; + } + + /** + * __d_path - return the path of a dentry + * @path: the dentry/vfsmount to report +- * @root: root vfsmnt/dentry (may be modified by this function) ++ * @root: root vfsmnt/dentry + * @buf: buffer to return value in + * @buflen: buffer length + * +@@ -2478,10 +2478,10 @@ global_root: + * + * "buflen" should be positive. + * +- * If path is not reachable from the supplied root, then the value of +- * root is changed (without modifying refcounts). ++ * If the path is not reachable from the supplied root, return %NULL. + */ +-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; +@@ -2492,7 +2492,28 @@ char *__d_path(const struct path *path, + error = prepend_path(path, root, &res, &buflen); + write_sequnlock(&rename_lock); + +- if (error) ++ if (error < 0) ++ return ERR_PTR(error); ++ if (error > 0) ++ return NULL; ++ return res; ++} ++ ++char *d_absolute_path(const struct path *path, ++ char *buf, int buflen) ++{ ++ struct path root = {}; ++ char *res = buf + buflen; ++ int error; ++ ++ prepend(&res, &buflen, "\0", 1); ++ write_seqlock(&rename_lock); ++ error = prepend_path(path, &root, &res, &buflen); ++ write_sequnlock(&rename_lock); ++ ++ if (error > 1) ++ error = -EINVAL; ++ if (error < 0) + return ERR_PTR(error); + return res; + } +@@ -2500,8 +2521,9 @@ char *__d_path(const struct path *path, + /* + * 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) ++static int path_with_deleted(const struct path *path, ++ const struct path *root, ++ char **buf, int *buflen) + { + prepend(buf, buflen, "\0", 1); + if (d_unlinked(path->dentry)) { +@@ -2538,7 +2560,6 @@ char *d_path(const struct path *path, ch + { + char *res = buf + buflen; + struct path root; +- struct path tmp; + int error; + + /* +@@ -2553,9 +2574,8 @@ char *d_path(const struct path *path, ch + + get_fs_root(current->fs, &root); + write_seqlock(&rename_lock); +- tmp = root; +- error = path_with_deleted(path, &tmp, &res, &buflen); +- if (error) ++ error = path_with_deleted(path, &root, &res, &buflen); ++ if (error < 0) + res = ERR_PTR(error); + write_sequnlock(&rename_lock); + path_put(&root); +@@ -2576,7 +2596,6 @@ char *d_path_with_unreachable(const stru + { + char *res = buf + buflen; + struct path root; +- struct path tmp; + int error; + + if (path->dentry->d_op && path->dentry->d_op->d_dname) +@@ -2584,9 +2603,8 @@ char *d_path_with_unreachable(const stru + + get_fs_root(current->fs, &root); + write_seqlock(&rename_lock); +- tmp = root; +- error = path_with_deleted(path, &tmp, &res, &buflen); +- if (!error && !path_equal(&tmp, &root)) ++ error = path_with_deleted(path, &root, &res, &buflen); ++ if (error > 0) + error = prepend_unreachable(&res, &buflen); + write_sequnlock(&rename_lock); + path_put(&root); +@@ -2717,19 +2735,18 @@ SYSCALL_DEFINE2(getcwd, char __user *, b + 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) ++ if (error < 0) + goto out; + + /* Unreachable from current root */ +- if (!path_equal(&tmp, &root)) { ++ if (error > 0) { + error = prepend_unreachable(&cwd, &buflen); + if (error) + goto out; +--- a/fs/namespace.c ++++ b/fs/namespace.c +@@ -1048,15 +1048,12 @@ static int show_mountinfo(struct seq_fil + if (err) + goto out; + seq_putc(m, ' '); +- seq_path_root(m, &mnt_path, &root, " \t\n\\"); +- if (root.mnt != p->root.mnt || root.dentry != p->root.dentry) { +- /* +- * Mountpoint is outside root, discard that one. Ugly, +- * but less so than trying to do that in iterator in a +- * race-free way (due to renames). +- */ +- return SEQ_SKIP; +- } ++ ++ /* mountpoints outside of chroot jail will give SEQ_SKIP on this */ ++ err = seq_path_root(m, &mnt_path, &root, " \t\n\\"); ++ if (err) ++ goto out; ++ + seq_puts(m, mnt->mnt_flags & MNT_READONLY ? " ro" : " rw"); + show_mnt_opts(m, mnt); + +@@ -2744,3 +2741,8 @@ void kern_unmount(struct vfsmount *mnt) + } + } + EXPORT_SYMBOL(kern_unmount); ++ ++bool our_mnt(struct vfsmount *mnt) ++{ ++ return check_mnt(mnt); ++} +--- a/fs/seq_file.c ++++ b/fs/seq_file.c +@@ -449,8 +449,6 @@ 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, + char *esc) +@@ -463,6 +461,8 @@ int seq_path_root(struct seq_file *m, st + char *p; + + p = __d_path(path, root, buf, size); ++ if (!p) ++ return SEQ_SKIP; + res = PTR_ERR(p); + if (!IS_ERR(p)) { + char *end = mangle_path(buf, p, esc); +@@ -474,7 +474,7 @@ int seq_path_root(struct seq_file *m, st + } + seq_commit(m, res); + +- return res < 0 ? res : 0; ++ return res < 0 && res != -ENAMETOOLONG ? res : 0; + } + + /* +--- a/include/linux/dcache.h ++++ b/include/linux/dcache.h +@@ -337,7 +337,8 @@ extern int d_validate(struct dentry *, s + */ + 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 *, const struct path *, char *, int); ++extern char *d_absolute_path(const struct path *, 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); +--- a/include/linux/fs.h ++++ b/include/linux/fs.h +@@ -1907,6 +1907,7 @@ extern int fd_statfs(int, struct kstatfs + extern int statfs_by_dentry(struct dentry *, struct kstatfs *); + extern int freeze_super(struct super_block *super); + extern int thaw_super(struct super_block *super); ++extern bool our_mnt(struct vfsmount *mnt); + + extern int current_umask(void); + +--- a/security/apparmor/path.c ++++ b/security/apparmor/path.c +@@ -57,23 +57,44 @@ static int prepend(char **buffer, int bu + static int d_namespace_path(struct path *path, char *buf, int buflen, + char **name, int flags) + { +- struct path root, tmp; + char *res; +- int connected, error = 0; ++ int error = 0; ++ int connected = 1; + +- /* Get the root we want to resolve too, released below */ ++ if (path->mnt->mnt_flags & MNT_INTERNAL) { ++ /* it's not mounted anywhere */ ++ res = dentry_path(path->dentry, buf, buflen); ++ *name = res; ++ if (IS_ERR(res)) { ++ *name = buf; ++ return PTR_ERR(res); ++ } ++ if (path->dentry->d_sb->s_magic == PROC_SUPER_MAGIC && ++ strncmp(*name, "/sys/", 5) == 0) { ++ /* TODO: convert over to using a per namespace ++ * control instead of hard coded /proc ++ */ ++ return prepend(name, *name - buf, "/proc", 5); ++ } ++ return 0; ++ } ++ ++ /* resolve paths relative to chroot?*/ + if (flags & PATH_CHROOT_REL) { +- /* resolve paths relative to chroot */ ++ struct path root; + get_fs_root(current->fs, &root); +- } else { +- /* resolve paths relative to namespace */ +- root.mnt = current->nsproxy->mnt_ns->root; +- root.dentry = root.mnt->mnt_root; +- path_get(&root); ++ res = __d_path(path, &root, buf, buflen); ++ if (res && !IS_ERR(res)) { ++ /* everything's fine */ ++ *name = res; ++ path_put(&root); ++ goto ok; ++ } ++ path_put(&root); ++ connected = 0; + } + +- tmp = root; +- res = __d_path(path, &tmp, buf, buflen); ++ res = d_absolute_path(path, buf, buflen); + + *name = res; + /* handle error conditions - and still allow a partial path to +@@ -84,7 +105,10 @@ static int d_namespace_path(struct path + *name = buf; + goto out; + } ++ if (!our_mnt(path->mnt)) ++ connected = 0; + ++ok: + /* Handle two cases: + * 1. A deleted dentry && profile is not allowing mediation of deleted + * 2. On some filesystems, newly allocated dentries appear to the +@@ -97,10 +121,7 @@ static int d_namespace_path(struct path + goto out; + } + +- /* Determine if the path is connected to the expected root */ +- connected = tmp.dentry == root.dentry && tmp.mnt == root.mnt; +- +- /* If the path is not connected, ++ /* If the path is not connected to the expected root, + * check if it is a sysctl and handle specially else remove any + * leading / that __d_path may have returned. + * Unless +@@ -112,17 +133,9 @@ static int d_namespace_path(struct path + * namespace root. + */ + if (!connected) { +- /* is the disconnect path a sysctl? */ +- if (tmp.dentry->d_sb->s_magic == PROC_SUPER_MAGIC && +- strncmp(*name, "/sys/", 5) == 0) { +- /* TODO: convert over to using a per namespace +- * control instead of hard coded /proc +- */ +- error = prepend(name, *name - buf, "/proc", 5); +- } else if (!(flags & PATH_CONNECT_PATH) && ++ if (!(flags & PATH_CONNECT_PATH) && + !(((flags & CHROOT_NSCONNECT) == CHROOT_NSCONNECT) && +- (tmp.mnt == current->nsproxy->mnt_ns->root && +- tmp.dentry == tmp.mnt->mnt_root))) { ++ our_mnt(path->mnt))) { + /* disconnected path, don't return pathname starting + * with '/' + */ +@@ -133,8 +146,6 @@ static int d_namespace_path(struct path + } + + out: +- path_put(&root); +- + return error; + } + +--- a/security/tomoyo/realpath.c ++++ b/security/tomoyo/realpath.c +@@ -83,9 +83,8 @@ static char *tomoyo_get_absolute_path(st + { + char *pos = ERR_PTR(-ENOMEM); + if (buflen >= 256) { +- struct path ns_root = { }; + /* go to whatever namespace root we are under */ +- pos = __d_path(path, &ns_root, buffer, buflen - 1); ++ pos = d_absolute_path(path, buffer, buflen - 1); + if (!IS_ERR(pos) && *pos == '/' && pos[1]) { + struct inode *inode = path->dentry->d_inode; + if (inode && S_ISDIR(inode->i_mode)) { diff --git a/queue-3.1/series b/queue-3.1/series index c5fb9eb3a58..8328c6a6feb 100644 --- a/queue-3.1/series +++ b/queue-3.1/series @@ -16,3 +16,5 @@ lockdep-kmemcheck-annotate-lock-in-lockdep_init_map.patch ptp-fix-clock_getres-implementation.patch mm-ensure-that-pfn_valid-is-called-once-per-pageblock-when-reserving-pageblocks.patch mm-vmalloc-check-for-page-allocation-failure-before-vmlist-insertion.patch +fix-apparmor-dereferencing-potentially-freed-dentry-sanitize-__d_path-api.patch +tomoyo-fix-pathname-handling-of-disconnected-paths.patch diff --git a/queue-3.1/tomoyo-fix-pathname-handling-of-disconnected-paths.patch b/queue-3.1/tomoyo-fix-pathname-handling-of-disconnected-paths.patch new file mode 100644 index 00000000000..17cc9d43749 --- /dev/null +++ b/queue-3.1/tomoyo-fix-pathname-handling-of-disconnected-paths.patch @@ -0,0 +1,45 @@ +From 1418a3e5ad4d01b1d4abf2c479c50b0cedd59e3f Mon Sep 17 00:00:00 2001 +From: Tetsuo Handa +Date: Thu, 8 Dec 2011 21:24:06 +0900 +Subject: TOMOYO: Fix pathname handling of disconnected paths. + +From: Tetsuo Handa + +commit 1418a3e5ad4d01b1d4abf2c479c50b0cedd59e3f upstream. + +Current tomoyo_realpath_from_path() implementation returns strange pathname +when calculating pathname of a file which belongs to lazy unmounted tree. +Use local pathname rather than strange absolute pathname in that case. + +Also, this patch fixes a regression by commit 02125a82 "fix apparmor +dereferencing potentially freed dentry, sanitize __d_path() API". + +Signed-off-by: Tetsuo Handa +Acked-by: Al Viro +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman + +--- + security/tomoyo/realpath.c | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +--- a/security/tomoyo/realpath.c ++++ b/security/tomoyo/realpath.c +@@ -275,8 +275,16 @@ char *tomoyo_realpath_from_path(struct p + pos = tomoyo_get_local_path(path->dentry, buf, + buf_len - 1); + /* Get absolute name for the rest. */ +- else ++ else { + pos = tomoyo_get_absolute_path(path, buf, buf_len - 1); ++ /* ++ * Fall back to local name if absolute name is not ++ * available. ++ */ ++ if (pos == ERR_PTR(-EINVAL)) ++ pos = tomoyo_get_local_path(path->dentry, buf, ++ buf_len - 1); ++ } + encode: + if (IS_ERR(pos)) + continue; -- 2.47.3