On 2/25/25 15:50, Ryan Lee wrote:
aa_dup_audit_data is called in check_user (file.c) with GFP_KERNEL, which is in turn called by aa_audit_file through path_name. GFP_KERNEL allocs may sleep, but the file permission hook that invokes aa_file_perm is called in an atomic context that doesn't allow sleeping:BUG: sleeping function called from invalid context at include/linux/sched/mm.h:337 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1821, name: 5 preempt_count: 1, expected: 0 RCU nest depth: 0, expected: 0 |3 locks held by 5/1821: |0: (&sig->cred_guard_mutex){....}-{3:3}, at: bprm_execve (fs/exec.c) |1: (&sig->exec_update_lock){....}-{3:3}, at: begin_new_exec (fs/exec.c) |2: (&newf->file_lock){....}-{2:2}, at: iterate_fd (fs/file.c) Call trace excerpt: aa_dup_audit_data (security/apparmor/audit.c) aa_audit_file (security/apparmor/file.c) ? srso_alias_return_thunk (arch/x86/lib/retpoline.S) path_name (security/apparmor/file.c) profile_path_perm (security/apparmor/file.c) aa_file_perm (security/apparmor/file.c) Switch the allocation flag for that call to GFP_ATOMIC instead. Signed-off-by: Ryan Lee <[email protected]>
NAK, this needs to be handled higher up the chain. We can push gfp info down into this fn, but switching this use atomic for everything because this specific use needs it isn't appropriate. Failure to allocate here can result in a denial so we really do not want to use atomic unless we have to.
--- security/apparmor/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 79e5307090e3..f7ccab51d416 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -142,7 +142,7 @@ static int check_user(struct aa_profile *profile, int err;/* assume we are going to dispatch */- node = aa_dup_audit_data(ad, GFP_KERNEL); + node = aa_dup_audit_data(ad, GFP_ATOMIC); if (!node) { AA_DEBUG(DEBUG_UPCALL, "notifcation failed to duplicate with error -ENOMEM\n");
