Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+53 -86
+5 -3
security/apparmor/domain.c
··· 357 357 AA_BUG(!ctx); 358 358 359 359 profile = aa_get_newest_profile(ctx->profile); 360 + 361 + /* buffer freed below, name is pointer into buffer */ 362 + get_buffers(buffer); 360 363 /* 361 364 * get the namespace from the replacement profile as replacement 362 365 * can change the namespace ··· 367 364 ns = profile->ns; 368 365 state = profile->file.start; 369 366 370 - /* buffer freed below, name is pointer into buffer */ 371 - error = aa_path_name(&bprm->file->f_path, profile->path_flags, &buffer, 367 + error = aa_path_name(&bprm->file->f_path, profile->path_flags, buffer, 372 368 &name, &info, profile->disconnected); 373 369 if (error) { 374 370 if (unconfined(profile) || ··· 517 515 cleanup: 518 516 aa_put_profile(new_profile); 519 517 aa_put_profile(profile); 520 - kfree(buffer); 518 + put_buffers(buffer); 521 519 522 520 return error; 523 521 }
+7 -6
security/apparmor/file.c
··· 285 285 int error; 286 286 287 287 flags |= profile->path_flags | (S_ISDIR(cond->mode) ? PATH_IS_DIR : 0); 288 - error = aa_path_name(path, flags, &buffer, &name, &info, 288 + get_buffers(buffer); 289 + error = aa_path_name(path, flags, buffer, &name, &info, 289 290 profile->disconnected); 290 291 if (error) { 291 292 if (error == -ENOENT && is_deleted(path->dentry)) { ··· 305 304 } 306 305 error = aa_audit_file(profile, &perms, op, request, name, NULL, 307 306 cond->uid, info, error); 308 - kfree(buffer); 307 + put_buffers(buffer); 309 308 310 309 return error; 311 310 } ··· 364 363 unsigned int state; 365 364 int error; 366 365 366 + get_buffers(buffer, buffer2); 367 367 lperms = nullperms; 368 368 369 369 /* buffer freed below, lname is pointer in buffer */ 370 - error = aa_path_name(&link, profile->path_flags, &buffer, &lname, 370 + error = aa_path_name(&link, profile->path_flags, buffer, &lname, 371 371 &info, profile->disconnected); 372 372 if (error) 373 373 goto audit; 374 374 375 375 /* buffer2 freed below, tname is pointer in buffer2 */ 376 - error = aa_path_name(&target, profile->path_flags, &buffer2, &tname, 376 + error = aa_path_name(&target, profile->path_flags, buffer2, &tname, 377 377 &info, profile->disconnected); 378 378 if (error) 379 379 goto audit; ··· 434 432 audit: 435 433 error = aa_audit_file(profile, &lperms, OP_LINK, request, 436 434 lname, tname, cond.uid, info, error); 437 - kfree(buffer); 438 - kfree(buffer2); 435 + put_buffers(buffer, buffer2); 439 436 440 437 return error; 441 438 }
+2 -2
security/apparmor/include/path.h
··· 23 23 PATH_CHROOT_NSCONNECT = 0x10, /* connect paths that are at ns root */ 24 24 25 25 PATH_DELEGATE_DELETED = 0x08000, /* delegate deleted files */ 26 - PATH_MEDIATE_DELETED = 0x10000, /* mediate deleted paths */ 26 + PATH_MEDIATE_DELETED = 0x10000, /* mediate deleted paths */ 27 27 }; 28 28 29 - int aa_path_name(const struct path *path, int flags, char **buffer, 29 + int aa_path_name(const struct path *path, int flags, char *buffer, 30 30 const char **name, const char **info, 31 31 const char *disconnected); 32 32
+39 -75
security/apparmor/path.c
··· 79 79 * d_namespace_path - lookup a name associated with a given path 80 80 * @path: path to lookup (NOT NULL) 81 81 * @buf: buffer to store path to (NOT NULL) 82 - * @buflen: length of @buf 83 82 * @name: Returns - pointer for start of path name with in @buf (NOT NULL) 84 83 * @flags: flags controlling path lookup 85 84 * @disconnected: string to prefix to disconnected paths ··· 89 90 * When no error the path name is returned in @name which points to 90 91 * to a position in @buf 91 92 */ 92 - static int d_namespace_path(const struct path *path, char *buf, int buflen, 93 - char **name, int flags, const char *disconnected) 93 + static int d_namespace_path(const struct path *path, char *buf, char **name, 94 + int flags, const char *disconnected) 94 95 { 95 96 char *res; 96 97 int error = 0; 97 98 int connected = 1; 99 + int isdir = (flags & PATH_IS_DIR) ? 1 : 0; 100 + int buflen = aa_g_path_max - isdir; 98 101 99 102 if (path->mnt->mnt_flags & MNT_INTERNAL) { 100 103 /* it's not mounted anywhere */ ··· 111 110 /* TODO: convert over to using a per namespace 112 111 * control instead of hard coded /proc 113 112 */ 114 - return prepend(name, *name - buf, "/proc", 5); 113 + error = prepend(name, *name - buf, "/proc", 5); 114 + goto out; 115 115 } else 116 - return disconnect(path, buf, name, flags, 117 - disconnected); 116 + error = disconnect(path, buf, name, flags, 117 + disconnected); 118 + goto out; 118 119 } 119 120 120 121 /* resolve paths relative to chroot?*/ ··· 135 132 * be returned. 136 133 */ 137 134 if (!res || IS_ERR(res)) { 138 - if (PTR_ERR(res) == -ENAMETOOLONG) 139 - return -ENAMETOOLONG; 135 + if (PTR_ERR(res) == -ENAMETOOLONG) { 136 + error = -ENAMETOOLONG; 137 + *name = buf; 138 + goto out; 139 + } 140 140 connected = 0; 141 141 res = dentry_path_raw(path->dentry, buf, buflen); 142 142 if (IS_ERR(res)) { ··· 152 146 153 147 *name = res; 154 148 149 + if (!connected) 150 + error = disconnect(path, buf, name, flags, disconnected); 151 + 155 152 /* Handle two cases: 156 153 * 1. A deleted dentry && profile is not allowing mediation of deleted 157 154 * 2. On some filesystems, newly allocated dentries appear to the ··· 162 153 * allocated. 163 154 */ 164 155 if (d_unlinked(path->dentry) && d_is_positive(path->dentry) && 165 - !(flags & PATH_MEDIATE_DELETED)) { 156 + !(flags & (PATH_MEDIATE_DELETED | PATH_DELEGATE_DELETED))) { 166 157 error = -ENOENT; 167 158 goto out; 168 159 } 169 160 170 - if (!connected) 171 - error = disconnect(path, buf, name, flags, disconnected); 172 - 173 161 out: 174 - return error; 175 - } 176 - 177 - /** 178 - * get_name_to_buffer - get the pathname to a buffer ensure dir / is appended 179 - * @path: path to get name for (NOT NULL) 180 - * @flags: flags controlling path lookup 181 - * @buffer: buffer to put name in (NOT NULL) 182 - * @size: size of buffer 183 - * @name: Returns - contains position of path name in @buffer (NOT NULL) 184 - * 185 - * Returns: %0 else error on failure 186 - */ 187 - static int get_name_to_buffer(const struct path *path, int flags, char *buffer, 188 - int size, char **name, const char **info, 189 - const char *disconnected) 190 - { 191 - int adjust = (flags & PATH_IS_DIR) ? 1 : 0; 192 - int error = d_namespace_path(path, buffer, size - adjust, name, flags, 193 - disconnected); 194 - 195 - if (!error && (flags & PATH_IS_DIR) && (*name)[1] != '\0') 196 - /* 197 - * Append "/" to the pathname. The root directory is a special 198 - * case; it already ends in slash. 199 - */ 200 - strcpy(&buffer[size - 2], "/"); 201 - 202 - if (info && error) { 203 - if (error == -ENOENT) 204 - *info = "Failed name lookup - deleted entry"; 205 - else if (error == -EACCES) 206 - *info = "Failed name lookup - disconnected path"; 207 - else if (error == -ENAMETOOLONG) 208 - *info = "Failed name lookup - name too long"; 209 - else 210 - *info = "Failed name lookup"; 211 - } 162 + /* 163 + * Append "/" to the pathname. The root directory is a special 164 + * case; it already ends in slash. 165 + */ 166 + if (!error && isdir && ((*name)[1] != '\0' || (*name)[0] != '/')) 167 + strcpy(&buf[aa_g_path_max - 2], "/"); 212 168 213 169 return error; 214 170 } 215 171 216 172 /** 217 - * aa_path_name - compute the pathname of a file 173 + * aa_path_name - get the pathname to a buffer ensure dir / is appended 218 174 * @path: path the file (NOT NULL) 219 175 * @flags: flags controlling path name generation 220 - * @buffer: buffer that aa_get_name() allocated (NOT NULL) 176 + * @buffer: buffer to put name in (NOT NULL) 221 177 * @name: Returns - the generated path name if !error (NOT NULL) 222 178 * @info: Returns - information on why the path lookup failed (MAYBE NULL) 223 179 * @disconnected: string to prepend to disconnected paths ··· 198 224 * 199 225 * Returns: %0 else error code if could retrieve name 200 226 */ 201 - int aa_path_name(const struct path *path, int flags, char **buffer, 227 + int aa_path_name(const struct path *path, int flags, char *buffer, 202 228 const char **name, const char **info, const char *disconnected) 203 229 { 204 - char *buf, *str = NULL; 205 - int size = 256; 206 - int error; 230 + char *str = NULL; 231 + int error = d_namespace_path(path, buffer, &str, flags, disconnected); 207 232 208 - *name = NULL; 209 - *buffer = NULL; 210 - for (;;) { 211 - /* freed by caller */ 212 - buf = kmalloc(size, GFP_KERNEL); 213 - if (!buf) 214 - return -ENOMEM; 215 - 216 - error = get_name_to_buffer(path, flags, buf, size, &str, info, 217 - disconnected); 218 - if (error != -ENAMETOOLONG) 219 - break; 220 - 221 - kfree(buf); 222 - size <<= 1; 223 - if (size > aa_g_path_max) 224 - return -ENAMETOOLONG; 225 - *info = NULL; 233 + if (info && error) { 234 + if (error == -ENOENT) 235 + *info = "Failed name lookup - deleted entry"; 236 + else if (error == -EACCES) 237 + *info = "Failed name lookup - disconnected path"; 238 + else if (error == -ENAMETOOLONG) 239 + *info = "Failed name lookup - name too long"; 240 + else 241 + *info = "Failed name lookup"; 226 242 } 227 - *buffer = buf; 243 + 228 244 *name = str; 229 245 230 246 return error;