apparmor: Move path lookup to using preallocated buffers

Dynamically allocating buffers is problematic and is an extra layer
that is a potntial point of failure and can slow down mediation.
Change path lookup to use the preallocated per cpu buffers.

Signed-off-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
John Johansen 2017-05-23 03:25:14 -07:00
parent 72c8a76864
commit 4227c333f6
4 changed files with 53 additions and 86 deletions

View File

@ -357,6 +357,9 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
AA_BUG(!ctx); AA_BUG(!ctx);
profile = aa_get_newest_profile(ctx->profile); profile = aa_get_newest_profile(ctx->profile);
/* buffer freed below, name is pointer into buffer */
get_buffers(buffer);
/* /*
* get the namespace from the replacement profile as replacement * get the namespace from the replacement profile as replacement
* can change the namespace * can change the namespace
@ -364,8 +367,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
ns = profile->ns; ns = profile->ns;
state = profile->file.start; state = profile->file.start;
/* buffer freed below, name is pointer into buffer */ error = aa_path_name(&bprm->file->f_path, profile->path_flags, buffer,
error = aa_path_name(&bprm->file->f_path, profile->path_flags, &buffer,
&name, &info, profile->disconnected); &name, &info, profile->disconnected);
if (error) { if (error) {
if (unconfined(profile) || if (unconfined(profile) ||
@ -515,7 +517,7 @@ audit:
cleanup: cleanup:
aa_put_profile(new_profile); aa_put_profile(new_profile);
aa_put_profile(profile); aa_put_profile(profile);
kfree(buffer); put_buffers(buffer);
return error; return error;
} }

View File

@ -285,7 +285,8 @@ int aa_path_perm(const char *op, struct aa_profile *profile,
int error; int error;
flags |= profile->path_flags | (S_ISDIR(cond->mode) ? PATH_IS_DIR : 0); flags |= profile->path_flags | (S_ISDIR(cond->mode) ? PATH_IS_DIR : 0);
error = aa_path_name(path, flags, &buffer, &name, &info, get_buffers(buffer);
error = aa_path_name(path, flags, buffer, &name, &info,
profile->disconnected); profile->disconnected);
if (error) { if (error) {
if (error == -ENOENT && is_deleted(path->dentry)) { if (error == -ENOENT && is_deleted(path->dentry)) {
@ -304,7 +305,7 @@ int aa_path_perm(const char *op, struct aa_profile *profile,
} }
error = aa_audit_file(profile, &perms, op, request, name, NULL, error = aa_audit_file(profile, &perms, op, request, name, NULL,
cond->uid, info, error); cond->uid, info, error);
kfree(buffer); put_buffers(buffer);
return error; return error;
} }
@ -363,16 +364,17 @@ int aa_path_link(struct aa_profile *profile, struct dentry *old_dentry,
unsigned int state; unsigned int state;
int error; int error;
get_buffers(buffer, buffer2);
lperms = nullperms; lperms = nullperms;
/* buffer freed below, lname is pointer in buffer */ /* buffer freed below, lname is pointer in buffer */
error = aa_path_name(&link, profile->path_flags, &buffer, &lname, error = aa_path_name(&link, profile->path_flags, buffer, &lname,
&info, profile->disconnected); &info, profile->disconnected);
if (error) if (error)
goto audit; goto audit;
/* buffer2 freed below, tname is pointer in buffer2 */ /* buffer2 freed below, tname is pointer in buffer2 */
error = aa_path_name(&target, profile->path_flags, &buffer2, &tname, error = aa_path_name(&target, profile->path_flags, buffer2, &tname,
&info, profile->disconnected); &info, profile->disconnected);
if (error) if (error)
goto audit; goto audit;
@ -432,8 +434,7 @@ done_tests:
audit: audit:
error = aa_audit_file(profile, &lperms, OP_LINK, request, error = aa_audit_file(profile, &lperms, OP_LINK, request,
lname, tname, cond.uid, info, error); lname, tname, cond.uid, info, error);
kfree(buffer); put_buffers(buffer, buffer2);
kfree(buffer2);
return error; return error;
} }

View File

@ -26,7 +26,7 @@ enum path_flags {
PATH_MEDIATE_DELETED = 0x10000, /* mediate deleted paths */ PATH_MEDIATE_DELETED = 0x10000, /* mediate deleted paths */
}; };
int aa_path_name(const struct path *path, int flags, char **buffer, int aa_path_name(const struct path *path, int flags, char *buffer,
const char **name, const char **info, const char **name, const char **info,
const char *disconnected); const char *disconnected);

View File

@ -79,7 +79,6 @@ static int disconnect(const struct path *path, char *buf, char **name,
* d_namespace_path - lookup a name associated with a given path * d_namespace_path - lookup a name associated with a given path
* @path: path to lookup (NOT NULL) * @path: path to lookup (NOT NULL)
* @buf: buffer to store path to (NOT NULL) * @buf: buffer to store path to (NOT NULL)
* @buflen: length of @buf
* @name: Returns - pointer for start of path name with in @buf (NOT NULL) * @name: Returns - pointer for start of path name with in @buf (NOT NULL)
* @flags: flags controlling path lookup * @flags: flags controlling path lookup
* @disconnected: string to prefix to disconnected paths * @disconnected: string to prefix to disconnected paths
@ -90,12 +89,14 @@ static int disconnect(const struct path *path, char *buf, char **name,
* When no error the path name is returned in @name which points to * When no error the path name is returned in @name which points to
* to a position in @buf * to a position in @buf
*/ */
static int d_namespace_path(const struct path *path, char *buf, int buflen, static int d_namespace_path(const struct path *path, char *buf, char **name,
char **name, int flags, const char *disconnected) int flags, const char *disconnected)
{ {
char *res; char *res;
int error = 0; int error = 0;
int connected = 1; int connected = 1;
int isdir = (flags & PATH_IS_DIR) ? 1 : 0;
int buflen = aa_g_path_max - isdir;
if (path->mnt->mnt_flags & MNT_INTERNAL) { if (path->mnt->mnt_flags & MNT_INTERNAL) {
/* it's not mounted anywhere */ /* it's not mounted anywhere */
@ -110,10 +111,12 @@ static int d_namespace_path(const struct path *path, char *buf, int buflen,
/* TODO: convert over to using a per namespace /* TODO: convert over to using a per namespace
* control instead of hard coded /proc * control instead of hard coded /proc
*/ */
return prepend(name, *name - buf, "/proc", 5); error = prepend(name, *name - buf, "/proc", 5);
goto out;
} else } else
return disconnect(path, buf, name, flags, error = disconnect(path, buf, name, flags,
disconnected); disconnected);
goto out;
} }
/* resolve paths relative to chroot?*/ /* resolve paths relative to chroot?*/
@ -132,8 +135,11 @@ static int d_namespace_path(const struct path *path, char *buf, int buflen,
* be returned. * be returned.
*/ */
if (!res || IS_ERR(res)) { if (!res || IS_ERR(res)) {
if (PTR_ERR(res) == -ENAMETOOLONG) if (PTR_ERR(res) == -ENAMETOOLONG) {
return -ENAMETOOLONG; error = -ENAMETOOLONG;
*name = buf;
goto out;
}
connected = 0; connected = 0;
res = dentry_path_raw(path->dentry, buf, buflen); res = dentry_path_raw(path->dentry, buf, buflen);
if (IS_ERR(res)) { if (IS_ERR(res)) {
@ -146,6 +152,9 @@ static int d_namespace_path(const struct path *path, char *buf, int buflen,
*name = res; *name = res;
if (!connected)
error = disconnect(path, buf, name, flags, disconnected);
/* Handle two cases: /* Handle two cases:
* 1. A deleted dentry && profile is not allowing mediation of deleted * 1. A deleted dentry && profile is not allowing mediation of deleted
* 2. On some filesystems, newly allocated dentries appear to the * 2. On some filesystems, newly allocated dentries appear to the
@ -153,62 +162,27 @@ static int d_namespace_path(const struct path *path, char *buf, int buflen,
* allocated. * allocated.
*/ */
if (d_unlinked(path->dentry) && d_is_positive(path->dentry) && if (d_unlinked(path->dentry) && d_is_positive(path->dentry) &&
!(flags & PATH_MEDIATE_DELETED)) { !(flags & (PATH_MEDIATE_DELETED | PATH_DELEGATE_DELETED))) {
error = -ENOENT; error = -ENOENT;
goto out; goto out;
} }
if (!connected)
error = disconnect(path, buf, name, flags, disconnected);
out: out:
return error;
}
/**
* get_name_to_buffer - get the pathname to a buffer ensure dir / is appended
* @path: path to get name for (NOT NULL)
* @flags: flags controlling path lookup
* @buffer: buffer to put name in (NOT NULL)
* @size: size of buffer
* @name: Returns - contains position of path name in @buffer (NOT NULL)
*
* Returns: %0 else error on failure
*/
static int get_name_to_buffer(const struct path *path, int flags, char *buffer,
int size, char **name, const char **info,
const char *disconnected)
{
int adjust = (flags & PATH_IS_DIR) ? 1 : 0;
int error = d_namespace_path(path, buffer, size - adjust, name, flags,
disconnected);
if (!error && (flags & PATH_IS_DIR) && (*name)[1] != '\0')
/* /*
* Append "/" to the pathname. The root directory is a special * Append "/" to the pathname. The root directory is a special
* case; it already ends in slash. * case; it already ends in slash.
*/ */
strcpy(&buffer[size - 2], "/"); if (!error && isdir && ((*name)[1] != '\0' || (*name)[0] != '/'))
strcpy(&buf[aa_g_path_max - 2], "/");
if (info && error) {
if (error == -ENOENT)
*info = "Failed name lookup - deleted entry";
else if (error == -EACCES)
*info = "Failed name lookup - disconnected path";
else if (error == -ENAMETOOLONG)
*info = "Failed name lookup - name too long";
else
*info = "Failed name lookup";
}
return error; return error;
} }
/** /**
* aa_path_name - compute the pathname of a file * aa_path_name - get the pathname to a buffer ensure dir / is appended
* @path: path the file (NOT NULL) * @path: path the file (NOT NULL)
* @flags: flags controlling path name generation * @flags: flags controlling path name generation
* @buffer: buffer that aa_get_name() allocated (NOT NULL) * @buffer: buffer to put name in (NOT NULL)
* @name: Returns - the generated path name if !error (NOT NULL) * @name: Returns - the generated path name if !error (NOT NULL)
* @info: Returns - information on why the path lookup failed (MAYBE NULL) * @info: Returns - information on why the path lookup failed (MAYBE NULL)
* @disconnected: string to prepend to disconnected paths * @disconnected: string to prepend to disconnected paths
@ -224,33 +198,23 @@ static int get_name_to_buffer(const struct path *path, int flags, char *buffer,
* *
* Returns: %0 else error code if could retrieve name * Returns: %0 else error code if could retrieve name
*/ */
int aa_path_name(const struct path *path, int flags, char **buffer, int aa_path_name(const struct path *path, int flags, char *buffer,
const char **name, const char **info, const char *disconnected) const char **name, const char **info, const char *disconnected)
{ {
char *buf, *str = NULL; char *str = NULL;
int size = 256; int error = d_namespace_path(path, buffer, &str, flags, disconnected);
int error;
*name = NULL; if (info && error) {
*buffer = NULL; if (error == -ENOENT)
for (;;) { *info = "Failed name lookup - deleted entry";
/* freed by caller */ else if (error == -EACCES)
buf = kmalloc(size, GFP_KERNEL); *info = "Failed name lookup - disconnected path";
if (!buf) else if (error == -ENAMETOOLONG)
return -ENOMEM; *info = "Failed name lookup - name too long";
else
error = get_name_to_buffer(path, flags, buf, size, &str, info, *info = "Failed name lookup";
disconnected);
if (error != -ENAMETOOLONG)
break;
kfree(buf);
size <<= 1;
if (size > aa_g_path_max)
return -ENAMETOOLONG;
*info = NULL;
} }
*buffer = buf;
*name = str; *name = str;
return error; return error;