apparmor: don't revalidate inherited files under files->file_lock

aa_inherit_files() revalidates inherited files on exec using
iterate_fd(), which calls the callback with files->file_lock held,
so the revalidation is done with IN_ATOMIC. However the unix socket
mediation path can sleep: aa_unix_file_perm() drops a ref on the
peer's path with path_put(), and unix_fs_perm() -> aa_path_perm()
allocates a buffer with aa_get_buffer(false), resulting in

  BUG: sleeping function called from invalid context at fs/dcache.c:1039
  ...
   dput.part.0+0x24/0x180
   path_put+0x2c/0x60
   aa_unix_file_perm+0x42a/0x17b0
   aa_file_perm+0xcd3/0x1060
   match_file+0x10b/0x190
   iterate_fd+0xe6/0x280
   aa_inherit_files+0x250/0x4f0

Only grab a ref on each file under iterate_fd(), and revalidate it
after the lock is dropped, so revalidation is done with no locks held
and does not need to be atomic. Denied fds are still replaced with
/dev/null, or closed if it can not be opened.

Fixes: b1aea2c196 ("apparmor: fix race in unix socket mediation when peer_path is used")
Cc: stable@vger.kernel.org
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
Georgia Garcia 2026-10-16 19:08:51 -03:00 committed by John Johansen
parent 4de32fd05c
commit a913281f60

View File

@ -706,49 +706,50 @@ static void revalidate_tty(const struct cred *subj_cred, struct aa_label *label)
no_tty(); no_tty();
} }
struct cred_label { /*
const struct cred *cred; * iterate_fd() callback, called with files->file_lock held: grab a ref
struct aa_label *label; * on the next file so it can be revalidated without the lock held.
}; */
static int get_next_file(const void *p, struct file *file, unsigned int fd)
static int match_file(const void *p, struct file *file, unsigned int fd)
{ {
struct cred_label *cl = (struct cred_label *)p; *(struct file **)p = get_file(file);
if (aa_file_perm(OP_INHERIT, cl->cred, cl->label, file, return fd + 1;
aa_map_file_to_perms(file), IN_ATOMIC))
return fd + 1;
return 0;
} }
/* based on selinux's flush_unauthorized_files */ /* based on selinux's flush_unauthorized_files */
void aa_inherit_files(const struct cred *cred, struct files_struct *files) void aa_inherit_files(const struct cred *cred, struct files_struct *files)
{ {
struct aa_label *label = aa_get_newest_cred_label(cred); struct aa_label *label = aa_get_newest_cred_label(cred);
struct cred_label cl = { struct file *devnull = NULL, *file;
.cred = cred, bool devnull_tried = false;
.label = label, unsigned int n = 0;
};
struct file *devnull = NULL;
unsigned int n;
revalidate_tty(cred, label); revalidate_tty(cred, label);
/* Revalidate access to inherited open files. */ /*
n = iterate_fd(files, 0, match_file, &cl); * Revalidate access to inherited open files. Each file is
if (!n) /* none found? */ * revalidated holding only a reference to it, with no locks
goto out; * held, as revalidation may sleep (e.g. unix socket mediation
* dropping path refs, or allocating path buffers).
*/
while ((n = iterate_fd(files, n, get_next_file, &file)) != 0) {
int error = aa_file_perm(OP_INHERIT, cred, label, file,
aa_map_file_to_perms(file), false);
devnull = dentry_open(&aa_null, O_RDWR, cred); fput(file);
if (IS_ERR(devnull)) if (!error)
devnull = NULL; continue;
/* replace all the matching ones with this */ if (!devnull_tried) {
do { devnull_tried = true;
devnull = dentry_open(&aa_null, O_RDWR, cred);
if (IS_ERR(devnull))
devnull = NULL;
}
/* replace the denied fd (n - 1) with devnull, or close it */
replace_fd(n - 1, devnull, 0); replace_fd(n - 1, devnull, 0);
} while ((n = iterate_fd(files, n, match_file, &cl)) != 0); }
if (devnull) if (devnull)
fput(devnull); fput(devnull);
out:
aa_put_label(label); aa_put_label(label);
} }