Commit 20d4e80d authored by John Johansen's avatar John Johansen

apparmor: only get a label reference if the fast path check fails

The common fast path check can be done under rcu_read_lock() and
doesn't need a reference count on the label. Only take a reference
count if entering the slow path.

Fixes reported hackbench regression
  - sha1 79e178a5 ("Merge tag 'apparmor-pr-2019-12-03' of
    git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor")

  hackbench -l (256000/#grp) -g #grp
   128 groups     19.679 ±0.90%

  - previous sha1 01d1dff6 ("Merge tag 's390-5.5-2' of
    git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux")

  hackbench -l (256000/#grp) -g #grp
   128 groups     3.1689 ±3.04%
Reported-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Tested-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Tested-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Fixes: bce4e7e9 ("apparmor: reduce rcu_read_lock scope for aa_file_perm mediation")
Signed-off-by: default avatarJohn Johansen <john.johansen@canonical.com>
parent 9c95a278
...@@ -618,8 +618,7 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, ...@@ -618,8 +618,7 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
fctx = file_ctx(file); fctx = file_ctx(file);
rcu_read_lock(); rcu_read_lock();
flabel = aa_get_newest_label(rcu_dereference(fctx->label)); flabel = rcu_dereference(fctx->label);
rcu_read_unlock();
AA_BUG(!flabel); AA_BUG(!flabel);
/* revalidate access, if task is unconfined, or the cached cred /* revalidate access, if task is unconfined, or the cached cred
...@@ -631,9 +630,13 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, ...@@ -631,9 +630,13 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
*/ */
denied = request & ~fctx->allow; denied = request & ~fctx->allow;
if (unconfined(label) || unconfined(flabel) || if (unconfined(label) || unconfined(flabel) ||
(!denied && aa_label_is_subset(flabel, label))) (!denied && aa_label_is_subset(flabel, label))) {
rcu_read_unlock();
goto done; goto done;
}
flabel = aa_get_newest_label(flabel);
rcu_read_unlock();
/* TODO: label cross check */ /* TODO: label cross check */
if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry)) if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry))
...@@ -643,8 +646,9 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, ...@@ -643,8 +646,9 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
else if (S_ISSOCK(file_inode(file)->i_mode)) else if (S_ISSOCK(file_inode(file)->i_mode))
error = __file_sock_perm(op, label, flabel, file, request, error = __file_sock_perm(op, label, flabel, file, request,
denied); denied);
done:
aa_put_label(flabel); aa_put_label(flabel);
done:
return error; return error;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment