From 28042fabf43b9a8ccfaa38f8c8187cc525e53fd3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 16 Feb 2012 06:21:30 -0800 Subject: AppArmor: Fix the error case for chroot relative path name lookup When a chroot relative pathname lookup fails it is falling through to do a d_absolute_path lookup. This is incorrect as d_absolute_path should only be used to lookup names for namespace absolute paths. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/path.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'security/apparmor/path.c') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index 9d070a7c3ffc..c31ce837fef4 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -91,9 +91,8 @@ static int d_namespace_path(struct path *path, char *buf, int buflen, } path_put(&root); connected = 0; - } - - res = d_absolute_path(path, buf, buflen); + } else + res = d_absolute_path(path, buf, buflen); *name = res; /* handle error conditions - and still allow a partial path to -- cgit v1.2.1 From ef9a762279c9ce98c592fb144b31898411feb94d Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 10 Mar 2012 11:19:51 -0800 Subject: AppArmor: Fix error returned when a path lookup is disconnected The returning of -ESATLE when a path lookup fails as disconnected is wrong. Since AppArmor is rejecting the access return -EACCES instead. This also fixes a bug in complain (learning) mode where disconnected paths are denied because -ESTALE errors are not ignored causing failures that can change application behavior. Signed-off-by: John Johansen --- security/apparmor/path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/apparmor/path.c') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index c31ce837fef4..3dd605c69970 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -137,7 +137,7 @@ ok: /* disconnected path, don't return pathname starting * with '/' */ - error = -ESTALE; + error = -EACCES; if (*res == '/') *name = res + 1; } -- cgit v1.2.1 From fbba8d89acea5d628d1d076b1d8962db438ff832 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 16 Feb 2012 06:28:50 -0800 Subject: AppArmor: Retrieve the dentry_path for error reporting when path lookup fails When __d_path and d_absolute_path fail due to the name being outside of the current namespace no name is reported. Use dentry_path to provide some hint as to which file was being accessed. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/path.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'security/apparmor/path.c') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index 3dd605c69970..8c90fd0f49c5 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -94,18 +94,21 @@ static int d_namespace_path(struct path *path, char *buf, int buflen, } else res = d_absolute_path(path, buf, buflen); - *name = res; /* handle error conditions - and still allow a partial path to * be returned. */ if (IS_ERR(res)) { - error = PTR_ERR(res); - *name = buf; - goto out; - } - if (!our_mnt(path->mnt)) + res = dentry_path_raw(path->dentry, buf, buflen); + if (IS_ERR(res)) { + error = PTR_ERR(res); + *name = buf; + goto out; + }; + } else if (!our_mnt(path->mnt)) connected = 0; + *name = res; + ok: /* Handle two cases: * 1. A deleted dentry && profile is not allowing mediation of deleted -- cgit v1.2.1 From 3372b68a3c982611dcc30b3c872f8bbdee019e5e Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 16 Feb 2012 06:32:47 -0800 Subject: AppArmor: Minor cleanup of d_namespace_path to consolidate error handling Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/path.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'security/apparmor/path.c') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index 8c90fd0f49c5..a7936dfe0e6c 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -83,21 +83,18 @@ static int d_namespace_path(struct path *path, char *buf, int buflen, struct path root; get_fs_root(current->fs, &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; - } else + } else { res = d_absolute_path(path, buf, buflen); + if (!our_mnt(path->mnt)) + connected = 0; + } /* handle error conditions - and still allow a partial path to * be returned. */ - if (IS_ERR(res)) { + if (!res || IS_ERR(res)) { + connected = 0; res = dentry_path_raw(path->dentry, buf, buflen); if (IS_ERR(res)) { error = PTR_ERR(res); @@ -109,7 +106,6 @@ static int d_namespace_path(struct path *path, char *buf, int buflen, *name = res; -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 -- cgit v1.2.1 From 57fa1e18091e66b7e1002816523cb218196a882e Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 16 Feb 2012 06:20:33 -0800 Subject: AppArmor: Move path failure information into aa_get_name and rename Move the path name lookup failure messages into the main path name lookup routine, as the information is useful in more than just aa_path_perm. Also rename aa_get_name to aa_path_name as it is not getting a reference counted object with a corresponding put fn. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/path.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'security/apparmor/path.c') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index a7936dfe0e6c..2daeea4f9266 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -157,7 +157,7 @@ out: * Returns: %0 else error on failure */ static int get_name_to_buffer(struct path *path, int flags, char *buffer, - int size, char **name) + int size, char **name, const char **info) { int adjust = (flags & PATH_IS_DIR) ? 1 : 0; int error = d_namespace_path(path, buffer, size - adjust, name, flags); @@ -169,15 +169,27 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer, */ strcpy(&buffer[size - 2], "/"); + if (info && error) { + if (error == -ENOENT) + *info = "Failed name lookup - deleted entry"; + else if (error == -ESTALE) + *info = "Failed name lookup - disconnected path"; + else if (error == -ENAMETOOLONG) + *info = "Failed name lookup - name too long"; + else + *info = "Failed name lookup"; + } + return error; } /** - * aa_get_name - compute the pathname of a file + * aa_path_name - compute the pathname of a file * @path: path the file (NOT NULL) * @flags: flags controlling path name generation * @buffer: buffer that aa_get_name() allocated (NOT NULL) * @name: Returns - the generated path name if !error (NOT NULL) + * @info: Returns - information on why the path lookup failed (MAYBE NULL) * * @name is a pointer to the beginning of the pathname (which usually differs * from the beginning of the buffer), or NULL. If there is an error @name @@ -190,7 +202,8 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer, * * Returns: %0 else error code if could retrieve name */ -int aa_get_name(struct path *path, int flags, char **buffer, const char **name) +int aa_path_name(struct path *path, int flags, char **buffer, const char **name, + const char **info) { char *buf, *str = NULL; int size = 256; @@ -204,7 +217,7 @@ int aa_get_name(struct path *path, int flags, char **buffer, const char **name) if (!buf) return -ENOMEM; - error = get_name_to_buffer(path, flags, buf, size, &str); + error = get_name_to_buffer(path, flags, buf, size, &str, info); if (error != -ENAMETOOLONG) break; @@ -212,6 +225,7 @@ int aa_get_name(struct path *path, int flags, char **buffer, const char **name) size <<= 1; if (size > aa_g_path_max) return -ENAMETOOLONG; + *info = NULL; } *buffer = buf; *name = str; -- cgit v1.2.1