aa_file_perm() passes an unreferenced RCU snapshot of fctx->label to aa_get_newest_label(). Its non-stale branch uses a plain kref_get(). Policy replacement can drop the proxy reference after the stale test while update_file_ctx() drops the cached reference, so pread64() can increment the label's refcount from zero. Acquire the RCU-published label with kref_get_unless_zero before testing it for staleness, keeping the label alive while resolving its replacement. refcount_t: addition on 0; use-after-free. WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x111/0x130, CPU#1: AAVICTIM/5091 RIP: refcount_warn_saturate (lib/refcount.c:25) Call Trace: aa_file_perm (security/apparmor/file.c:657) common_file_perm (security/apparmor/lsm.c:529) security_file_permission (security/security.c:2392) rw_verify_area (fs/read_write.c:476) vfs_read (fs/read_write.c:566) __x64_sys_pread64 (fs/read_write.c:770) do_syscall_64 (arch/x86/entry/syscall_64.c:94) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) Fixes: bce4e7e9c45e ("apparmor: reduce rcu_read_lock scope for aa_file_perm mediation") Cc: stable@vger.kernel.org Reported-by: Signed-off-by: Weiming Shi --- security/apparmor/file.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 694e157149e85e..59c27b9c5a8d15 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -621,7 +621,7 @@ int aa_file_perm(const char *op, const struct cred *subj_cred, u32 request, bool in_atomic) { struct aa_file_ctx *fctx; - struct aa_label *flabel; + struct aa_label *flabel, *tmp; u32 denied; int error = 0; @@ -654,7 +654,12 @@ int aa_file_perm(const char *op, const struct cred *subj_cred, } /* slow path - revalidate access */ - flabel = aa_get_newest_label(flabel); + flabel = aa_get_label_rcu(&fctx->label); + if (label_is_stale(flabel)) { + tmp = flabel; + flabel = aa_get_newest_label(tmp); + aa_put_label(tmp); + } rcu_read_unlock(); if (path_mediated_fs(file->f_path.dentry)) -- 2.56.0