summaryrefslogtreecommitdiff
path: root/security/apparmor
AgeCommit message (Collapse)AuthorFilesLines
10 daysMerge tag 'apparmor-pr-2026-04-23' of ↵Linus Torvalds4-26/+22
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor Pull apparmor updates from John Johansen: "Cleanups - Use sysfs_emit in param_get_{audit,mode} - Remove redundant if check in sk_peer_get_label - Replace memcpy + NUL termination with kmemdup_nul in do_setattr Bug Fixes: - Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine - Fix string overrun due to missing termination - Fix wrong dentry in RENAME_EXCHANGE uid check - fix unpack_tags to properly return error in failure cases - fix dfa size check - return error on namespace mismatch in verify_header - use target task's context in apparmor_getprocattr()" * tag 'apparmor-pr-2026-04-23' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: apparmor/lsm: Fix aa_dfa_unpack's error handling in aa_setup_dfa_engine apparmor: Fix string overrun due to missing termination apparmor: Fix wrong dentry in RENAME_EXCHANGE uid check apparmor: fix unpack_tags to properly return error in failure cases apparmor: fix dfa size check apparmor: Use sysfs_emit in param_get_{audit,mode} apparmor: Remove redundant if check in sk_peer_get_label apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr apparmor: return error on namespace mismatch in verify_header apparmor: use target task's context in apparmor_getprocattr()
12 daysapparmor/lsm: Fix aa_dfa_unpack's error handling in aa_setup_dfa_engineGONG Ruiqi1-0/+1
aa_dfa_unpack returns ERR_PTR not NULL when it fails, but aa_put_dfa only checks NULL for its input, which would cause invalid memory access in aa_put_dfa. Set nulldfa to NULL explicitly to fix that. Fixes: 98b824ff8984 ("apparmor: refcount the pdb") Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: Fix string overrun due to missing terminationDaniel J Blueman1-3/+5
When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm Snapdragon X1 we see a string buffer overrun: BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535) Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120 CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025 Call trace: show_stack (arch/arm64/kernel/stacktrace.c:501) (C) dump_stack_lvl (lib/dump_stack.c:122) print_report (mm/kasan/report.c:379 mm/kasan/report.c:482) kasan_report (mm/kasan/report.c:597) __asan_report_load1_noabort (mm/kasan/report_generic.c:378) aa_dfa_match (security/apparmor/match.c:535) match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336) match_mnt (security/apparmor/mount.c:371) aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4)) apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1)) security_sb_mount (security/security.c:1062 (discriminator 31)) path_mount (fs/namespace.c:4101) __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338) invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49) el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2)) do_el0_svc (arch/arm64/kernel/syscall.c:152) el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725) el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744) el0t_64_sync (arch/arm64/kernel/entry.S:596) Allocated by task 2120: kasan_save_stack (mm/kasan/common.c:58) kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79) kasan_save_alloc_info (mm/kasan/generic.c:571) __kasan_kmalloc (mm/kasan/common.c:419) __kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272) aa_get_buffer (security/apparmor/lsm.c:2201) aa_bind_mount (security/apparmor/mount.c:442) apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1)) security_sb_mount (security/security.c:1062 (discriminator 31)) path_mount (fs/namespace.c:4101) __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338) invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49) el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2)) do_el0_svc (arch/arm64/kernel/syscall.c:152) el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725) el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744) el0t_64_sync (arch/arm64/kernel/entry.S:596) The buggy address belongs to the object at ffff0008901ca000 which belongs to the cache kmalloc-rnd-06-8k of size 8192 The buggy address is located 0 bytes to the right of allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000) The buggy address belongs to the physical page: page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8 head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0 flags: 0x8000000000000040(head|zone=2) page_type: f5(slab) raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70 raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000 head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70 head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000 head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ^ ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc This was introduced by previous incorrect conversion from strcpy(). Fix it by adding the missing terminator. Cc: stable@vger.kernel.org Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: Daniel J Blueman <daniel@quora.org> Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path") Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: Fix wrong dentry in RENAME_EXCHANGE uid checkDudu Lu1-1/+1
In apparmor_path_rename(), when handling RENAME_EXCHANGE, the cond_exchange structure is supposed to carry the attributes of the *new* dentry (since it is used to authorize moving new_dentry to the old location). However, line 412 reads: vfsuid = i_uid_into_vfsuid(idmap, d_backing_inode(old_dentry)); This fetches the uid of old_dentry instead of new_dentry. As a result, the RENAME_EXCHANGE permission check uses the wrong file owner, which can allow a rename that should be denied (if old_dentry's owner has more privileges) or deny one that should be allowed. Note that cond_exchange.mode on the line above correctly uses new_dentry. Only the uid lookup is wrong. Fix by changing old_dentry to new_dentry in the i_uid_into_vfsuid call. Fixes: 5e26a01e56fd ("apparmor: use type safe idmapping helpers") Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: Dudu Lu <phx0fer@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: fix unpack_tags to properly return error in failure casesJohn Johansen1-0/+1
error is initialized to -EPROTO but set by some of the internal functions, unfortunately the last two checks assume error is set to -EPROTO already for the failure case. Ensure it is by setting it before these checks. Fixes: 3d28e2397af7a ("apparmor: add support loading per permission tagging") Reported-by: Dan Carpenter <error27@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: fix dfa size checkJohn Johansen1-1/+1
AppArmor dfas need a minimum of two states to be valid. State 0 is the default trap state, and State 1 the default start state. When verifying the dfa ensure that this is the case. Fixes: c27c6bd2c4d6b ("apparmor: ensure that dfa state tables have entries") Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: Use sysfs_emit in param_get_{audit,mode}Thorsten Blum1-3/+3
Replace sprintf() with sysfs_emit() in param_get_audit() and param_get_mode(). sysfs_emit() is preferred for formatting sysfs output because it provides safer bounds checking. Add terminating newlines as suggested by checkpatch. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: Remove redundant if check in sk_peer_get_labelThorsten Blum1-5/+1
Remove the redundant if check in sk_peer_get_label() and return ERR_PTR(-ENOPROTOOPT) directly. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Signed-off-by: John Johansen <john.johansen@canonical.com>
12 daysapparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattrThorsten Blum1-4/+1
Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by a manual NUL termination. No functional changes. Reviewed-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-04-13Merge tag 'vfs-7.1-rc1.kino' of ↵Linus Torvalds1-2/+2
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs Pull vfs i_ino updates from Christian Brauner: "For historical reasons, the inode->i_ino field is an unsigned long, which means that it's 32 bits on 32 bit architectures. This has caused a number of filesystems to implement hacks to hash a 64-bit identifier into a 32-bit field, and deprives us of a universal identifier field for an inode. This changes the inode->i_ino field from an unsigned long to a u64. This shouldn't make any material difference on 64-bit hosts, but 32-bit hosts will see struct inode grow by at least 4 bytes. This could have effects on slabcache sizes and field alignment. The bulk of the changes are to format strings and tracepoints, since the kernel itself doesn't care that much about the i_ino field. The first patch changes some vfs function arguments, so check that one out carefully. With this change, we may be able to shrink some inode structures. For instance, struct nfs_inode has a fileid field that holds the 64-bit inode number. With this set of changes, that field could be eliminated. I'd rather leave that sort of cleanups for later just to keep this simple" * tag 'vfs-7.1-rc1.kino' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: nilfs2: fix 64-bit division operations in nilfs_bmap_find_target_in_group() EVM: add comment describing why ino field is still unsigned long vfs: remove externs from fs.h on functions modified by i_ino widening treewide: fix missed i_ino format specifier conversions ext4: fix signed format specifier in ext4_load_inode trace event treewide: change inode->i_ino from unsigned long to u64 nilfs2: widen trace event i_ino fields to u64 f2fs: widen trace event i_ino fields to u64 ext4: widen trace event i_ino fields to u64 zonefs: widen trace event i_ino fields to u64 hugetlbfs: widen trace event i_ino fields to u64 ext2: widen trace event i_ino fields to u64 cachefiles: widen trace event i_ino fields to u64 vfs: widen trace event i_ino fields to u64 net: change sock.sk_ino and sock_i_ino() to u64 audit: widen ino fields to u64 vfs: widen inode hash/lookup functions to u64
2026-04-13Merge tag 'vfs-7.1-rc1.directory' of ↵Linus Torvalds1-26/+8
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs Pull vfs directory updates from Christian Brauner: "Recently 'start_creating', 'start_removing', 'start_renaming' and related interfaces were added which combine the locking and the lookup. At that time many callers were changed to use the new interfaces. However there are still an assortment of places out side of the core vfs where the directory is locked explictly, whether with inode_lock() or lock_rename() or similar. These were missed in the first pass for an assortment of uninteresting reasons. This addresses the remaining places where explicit locking is used, and changes them to use the new interfaces, or otherwise removes the explicit locking. The biggest changes are in overlayfs. The other changes are quite simple, though maybe the cachefiles changes is the least simple of those" * tag 'vfs-7.1-rc1.directory' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: VFS: unexport lock_rename(), lock_rename_child(), unlock_rename() ovl: remove ovl_lock_rename_workdir() ovl: use is_subdir() for testing if one thing is a subdir of another ovl: change ovl_create_real() to get a new lock when re-opening created file. ovl: pass name buffer to ovl_start_creating_temp() cachefiles: change cachefiles_bury_object to use start_renaming_dentry() ovl: Simplify ovl_lookup_real_one() VFS: make lookup_one_qstr_excl() static. nfsd: switch purge_old() to use start_removing_noperm() selinux: Use simple_start_creating() / simple_done_creating() Apparmor: Use simple_start_creating() / simple_done_creating() libfs: change simple_done_creating() to use end_creating() VFS: move the start_dirop() kerndoc comment to before start_dirop() fs/proc: Don't lock root inode when creating "self" and "thread-self" VFS: note error returns in documentation for various lookup functions
2026-03-10apparmor: fix race between freeing data and fs accessing itJohn Johansen7-101/+153
AppArmor was putting the reference to i_private data on its end after removing the original entry from the file system. However the inode can aand does live beyond that point and it is possible that some of the fs call back functions will be invoked after the reference has been put, which results in a race between freeing the data and accessing it through the fs. While the rawdata/loaddata is the most likely candidate to fail the race, as it has the fewest references. If properly crafted it might be possible to trigger a race for the other types stored in i_private. Fix this by moving the put of i_private referenced data to the correct place which is during inode eviction. Fixes: c961ee5f21b20 ("apparmor: convert from securityfs to apparmorfs for policy ns files") Reported-by: Qualys Security Advisory <qsa@qualys.com> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Maxime Bélair <maxime.belair@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix race on rawdata dereferenceJohn Johansen4-57/+93
There is a race condition that leads to a use-after-free situation: because the rawdata inodes are not refcounted, an attacker can start open()ing one of the rawdata files, and at the same time remove the last reference to this rawdata (by removing the corresponding profile, for example), which frees its struct aa_loaddata; as a result, when seq_rawdata_open() is reached, i_private is a dangling pointer and freed memory is accessed. The rawdata inodes weren't refcounted to avoid a circular refcount and were supposed to be held by the profile rawdata reference. However during profile removal there is a window where the vfs and profile destruction race, resulting in the use after free. Fix this by moving to a double refcount scheme. Where the profile refcount on rawdata is used to break the circular dependency. Allowing for freeing of the rawdata once all inode references to the rawdata are put. Fixes: 5d5182cae401 ("apparmor: move to per loaddata files, instead of replicating in profiles") Reported-by: Qualys Security Advisory <qsa@qualys.com> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Maxime Bélair <maxime.belair@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix differential encoding verificationJohn Johansen2-4/+20
Differential encoding allows loops to be created if it is abused. To prevent this the unpack should verify that a diff-encode chain terminates. Unfortunately the differential encode verification had two bugs. 1. it conflated states that had gone through check and already been marked, with states that were currently being checked and marked. This means that loops in the current chain being verified are treated as a chain that has already been verified. 2. the order bailout on already checked states compared current chain check iterators j,k instead of using the outer loop iterator i. Meaning a step backwards in states in the current chain verification was being mistaken for moving to an already verified state. Move to a double mark scheme where already verified states get a different mark, than the current chain being kept. This enables us to also drop the backwards verification check that was the cause of the second error as any already verified state is already marked. Fixes: 031dcc8f4e84 ("apparmor: dfa add support for state differential encoding") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix unprivileged local user can do privileged policy managementJohn Johansen3-9/+43
An unprivileged local user can load, replace, and remove profiles by opening the apparmorfs interfaces, via a confused deputy attack, by passing the opened fd to a privileged process, and getting the privileged process to write to the interface. This does require a privileged target that can be manipulated to do the write for the unprivileged process, but once such access is achieved full policy management is possible and all the possible implications that implies: removing confinement, DoS of system or target applications by denying all execution, by-passing the unprivileged user namespace restriction, to exploiting kernel bugs for a local privilege escalation. The policy management interface can not have its permissions simply changed from 0666 to 0600 because non-root processes need to be able to load policy to different policy namespaces. Instead ensure the task writing the interface has privileges that are a subset of the task that opened the interface. This is already done via policy for confined processes, but unconfined can delegate access to the opened fd, by-passing the usual policy check. Fixes: b7fd2c0340eac ("apparmor: add per policy ns .load, .replace, .remove interface files") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: Fix double free of ns_name in aa_replace_profiles()John Johansen1-0/+1
if ns_name is NULL after 1071 error = aa_unpack(udata, &lh, &ns_name); and if ent->ns_name contains an ns_name in 1089 } else if (ent->ns_name) { then ns_name is assigned the ent->ns_name 1095 ns_name = ent->ns_name; however ent->ns_name is freed at 1262 aa_load_ent_free(ent); and then again when freeing ns_name at 1270 kfree(ns_name); Fix this by NULLing out ent->ns_name after it is transferred to ns_name Fixes: 145a0ef21c8e9 ("apparmor: fix blob compression when ns is forced on a policy load ") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix missing bounds check on DEFAULT table in verify_dfa()Massimiliano Pellizzer1-2/+3
The verify_dfa() function only checks DEFAULT_TABLE bounds when the state is not differentially encoded. When the verification loop traverses the differential encoding chain, it reads k = DEFAULT_TABLE[j] and uses k as an array index without validation. A malformed DFA with DEFAULT_TABLE[j] >= state_count, therefore, causes both out-of-bounds reads and writes. [ 57.179855] ================================================================== [ 57.180549] BUG: KASAN: slab-out-of-bounds in verify_dfa+0x59a/0x660 [ 57.180904] Read of size 4 at addr ffff888100eadec4 by task su/993 [ 57.181554] CPU: 1 UID: 0 PID: 993 Comm: su Not tainted 6.19.0-rc7-next-20260127 #1 PREEMPT(lazy) [ 57.181558] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 57.181563] Call Trace: [ 57.181572] <TASK> [ 57.181577] dump_stack_lvl+0x5e/0x80 [ 57.181596] print_report+0xc8/0x270 [ 57.181605] ? verify_dfa+0x59a/0x660 [ 57.181608] kasan_report+0x118/0x150 [ 57.181620] ? verify_dfa+0x59a/0x660 [ 57.181623] verify_dfa+0x59a/0x660 [ 57.181627] aa_dfa_unpack+0x1610/0x1740 [ 57.181629] ? __kmalloc_cache_noprof+0x1d0/0x470 [ 57.181640] unpack_pdb+0x86d/0x46b0 [ 57.181647] ? srso_alias_return_thunk+0x5/0xfbef5 [ 57.181653] ? srso_alias_return_thunk+0x5/0xfbef5 [ 57.181656] ? aa_unpack_nameX+0x1a8/0x300 [ 57.181659] aa_unpack+0x20b0/0x4c30 [ 57.181662] ? srso_alias_return_thunk+0x5/0xfbef5 [ 57.181664] ? stack_depot_save_flags+0x33/0x700 [ 57.181681] ? kasan_save_track+0x4f/0x80 [ 57.181683] ? kasan_save_track+0x3e/0x80 [ 57.181686] ? __kasan_kmalloc+0x93/0xb0 [ 57.181688] ? __kvmalloc_node_noprof+0x44a/0x780 [ 57.181693] ? aa_simple_write_to_buffer+0x54/0x130 [ 57.181697] ? policy_update+0x154/0x330 [ 57.181704] aa_replace_profiles+0x15a/0x1dd0 [ 57.181707] ? srso_alias_return_thunk+0x5/0xfbef5 [ 57.181710] ? __kvmalloc_node_noprof+0x44a/0x780 [ 57.181712] ? aa_loaddata_alloc+0x77/0x140 [ 57.181715] ? srso_alias_return_thunk+0x5/0xfbef5 [ 57.181717] ? _copy_from_user+0x2a/0x70 [ 57.181730] policy_update+0x17a/0x330 [ 57.181733] profile_replace+0x153/0x1a0 [ 57.181735] ? rw_verify_area+0x93/0x2d0 [ 57.181740] vfs_write+0x235/0xab0 [ 57.181745] ksys_write+0xb0/0x170 [ 57.181748] do_syscall_64+0x8e/0x660 [ 57.181762] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 57.181765] RIP: 0033:0x7f6192792eb2 Remove the MATCH_FLAG_DIFF_ENCODE condition to validate all DEFAULT_TABLE entries unconditionally. Fixes: 031dcc8f4e84 ("apparmor: dfa add support for state differential encoding") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix side-effect bug in match_char() macro usageMassimiliano Pellizzer1-10/+20
The match_char() macro evaluates its character parameter multiple times when traversing differential encoding chains. When invoked with *str++, the string pointer advances on each iteration of the inner do-while loop, causing the DFA to check different characters at each iteration and therefore skip input characters. This results in out-of-bounds reads when the pointer advances past the input buffer boundary. [ 94.984676] ================================================================== [ 94.985301] BUG: KASAN: slab-out-of-bounds in aa_dfa_match+0x5ae/0x760 [ 94.985655] Read of size 1 at addr ffff888100342000 by task file/976 [ 94.986319] CPU: 7 UID: 1000 PID: 976 Comm: file Not tainted 6.19.0-rc7-next-20260127 #1 PREEMPT(lazy) [ 94.986322] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 94.986329] Call Trace: [ 94.986341] <TASK> [ 94.986347] dump_stack_lvl+0x5e/0x80 [ 94.986374] print_report+0xc8/0x270 [ 94.986384] ? aa_dfa_match+0x5ae/0x760 [ 94.986388] kasan_report+0x118/0x150 [ 94.986401] ? aa_dfa_match+0x5ae/0x760 [ 94.986405] aa_dfa_match+0x5ae/0x760 [ 94.986408] __aa_path_perm+0x131/0x400 [ 94.986418] aa_path_perm+0x219/0x2f0 [ 94.986424] apparmor_file_open+0x345/0x570 [ 94.986431] security_file_open+0x5c/0x140 [ 94.986442] do_dentry_open+0x2f6/0x1120 [ 94.986450] vfs_open+0x38/0x2b0 [ 94.986453] ? may_open+0x1e2/0x2b0 [ 94.986466] path_openat+0x231b/0x2b30 [ 94.986469] ? __x64_sys_openat+0xf8/0x130 [ 94.986477] do_file_open+0x19d/0x360 [ 94.986487] do_sys_openat2+0x98/0x100 [ 94.986491] __x64_sys_openat+0xf8/0x130 [ 94.986499] do_syscall_64+0x8e/0x660 [ 94.986515] ? count_memcg_events+0x15f/0x3c0 [ 94.986526] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986540] ? handle_mm_fault+0x1639/0x1ef0 [ 94.986551] ? vma_start_read+0xf0/0x320 [ 94.986558] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986561] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986563] ? fpregs_assert_state_consistent+0x50/0xe0 [ 94.986572] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986574] ? arch_exit_to_user_mode_prepare+0x9/0xb0 [ 94.986587] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986588] ? irqentry_exit+0x3c/0x590 [ 94.986595] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 94.986597] RIP: 0033:0x7fda4a79c3ea Fix by extracting the character value before invoking match_char, ensuring single evaluation per outer loop. Fixes: 074c1cd798cb ("apparmor: dfa move character match into a macro") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix: limit the number of levels of policy namespacesJohn Johansen2-0/+4
Currently the number of policy namespaces is not bounded relying on the user namespace limit. However policy namespaces aren't strictly tied to user namespaces and it is possible to create them and nest them arbitrarily deep which can be used to exhaust system resource. Hard cap policy namespaces to the same depth as user namespaces. Fixes: c88d4c7b049e8 ("AppArmor: core policy routines") Reported-by: Qualys Security Advisory <qsa@qualys.com> Reviewed-by: Ryan Lee <ryan.lee@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: replace recursive profile removal with iterative approachMassimiliano Pellizzer1-3/+27
The profile removal code uses recursion when removing nested profiles, which can lead to kernel stack exhaustion and system crashes. Reproducer: $ pf='a'; for ((i=0; i<1024; i++)); do echo -e "profile $pf { \n }" | apparmor_parser -K -a; pf="$pf//x"; done $ echo -n a > /sys/kernel/security/apparmor/.remove Replace the recursive __aa_profile_list_release() approach with an iterative approach in __remove_profile(). The function repeatedly finds and removes leaf profiles until the entire subtree is removed, maintaining the same removal semantic without recursion. Fixes: c88d4c7b049e ("AppArmor: core policy routines") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: fix memory leak in verify_headerMassimiliano Pellizzer1-1/+0
The function sets `*ns = NULL` on every call, leaking the namespace string allocated in previous iterations when multiple profiles are unpacked. This also breaks namespace consistency checking since *ns is always NULL when the comparison is made. Remove the incorrect assignment. The caller (aa_unpack) initializes *ns to NULL once before the loop, which is sufficient. Fixes: dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-10apparmor: validate DFA start states are in bounds in unpack_pdbMassimiliano Pellizzer1-1/+11
Start states are read from untrusted data and used as indexes into the DFA state tables. The aa_dfa_next() function call in unpack_pdb() will access dfa->tables[YYTD_ID_BASE][start], and if the start state exceeds the number of states in the DFA, this results in an out-of-bound read. ================================================================== BUG: KASAN: slab-out-of-bounds in aa_dfa_next+0x2a1/0x360 Read of size 4 at addr ffff88811956fb90 by task su/1097 ... Reject policies with out-of-bounds start states during unpacking to prevent the issue. Fixes: ad5ff3db53c6 ("AppArmor: Add ability to load extended policy") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-03-06treewide: change inode->i_ino from unsigned long to u64Jeff Layton1-2/+2
On 32-bit architectures, unsigned long is only 32 bits wide, which causes 64-bit inode numbers to be silently truncated. Several filesystems (NFS, XFS, BTRFS, etc.) can generate inode numbers that exceed 32 bits, and this truncation can lead to inode number collisions and other subtle bugs on 32-bit systems. Change the type of inode->i_ino from unsigned long to u64 to ensure that inode numbers are always represented as 64-bit values regardless of architecture. Update all format specifiers treewide from %lu/%lx to %llu/%llx to match the new type, along with corresponding local variable types. This is the bulk treewide conversion. Earlier patches in this series handled trace events separately to allow trace field reordering for better struct packing on 32-bit. Signed-off-by: Jeff Layton <jlayton@kernel.org> Link: https://patch.msgid.link/20260304-iino-u64-v3-12-2257ad83d372@kernel.org Acked-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Jan Kara <jack@suse.cz> Reviewed-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Christian Brauner <brauner@kernel.org>
2026-03-06Apparmor: Use simple_start_creating() / simple_done_creating()NeilBrown1-27/+8
Instead of explicitly locking the parent and performing a look up in apparmor, use simple_start_creating(), and then simple_done_creating() to unlock and drop the dentry. This removes the need to check for an existing entry (as simple_start_creating() acts like an exclusive create and can return -EEXIST), simplifies error paths, and keeps dir locking code centralised. Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: NeilBrown <neil@brown.name> Link: https://patch.msgid.link/20260224222542.3458677-6-neilb@ownmail.net Signed-off-by: Christian Brauner <brauner@kernel.org>
2026-02-24apparmor: return error on namespace mismatch in verify_headerMassimiliano Pellizzer1-0/+1
When profiles in a multi-profile load specify different namesapaces, the audit record is generated but execution continues, causing the function to return success. This violates the load requirement that all profiles must target the same namespace. Add the missing return statement after auditing the error. Reported-by: Qualys Security Advisory <qsa@qualys.com> Fixes: dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once") Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-24apparmor: use target task's context in apparmor_getprocattr()Cengiz Can1-9/+7
apparmor_getprocattr() incorrectly calls task_ctx(current) instead of task_ctx(task) when retrieving prev and exec attributes, returning the caller's labels rather than the target's. Fix by passing task to task_ctx(). The issue can be reproduced when a process with an onexec transition (e.g., configured by a container runtime) is inspected via /proc/<pid>/attr/apparmor/exec. The reader's own value is returned instead of the target's. Reported-by: Qualys Security Advisory <qsa@qualys.com> Fixes: 3b529a7600d8 ("apparmor: move task domain change info to task security") Cc: stable@vger.kernel.org Co-developed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Cengiz Can <cengiz.can@canonical.com> Co-developed-by: John Johansen <john.johansen@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-22Convert more 'alloc_obj' cases to default GFP_KERNEL argumentsLinus Torvalds1-2/+1
This converts some of the visually simpler cases that have been split over multiple lines. I only did the ones that are easy to verify the resulting diff by having just that final GFP_KERNEL argument on the next line. Somebody should probably do a proper coccinelle script for this, but for me the trivial script actually resulted in an assertion failure in the middle of the script. I probably had made it a bit _too_ trivial. So after fighting that far a while I decided to just do some of the syntactically simpler cases with variations of the previous 'sed' scripts. The more syntactically complex multi-line cases would mostly really want whitespace cleanup anyway. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2026-02-22Convert 'alloc_obj' family to use the new default GFP_KERNEL argumentLinus Torvalds6-14/+14
This was done entirely with mindless brute force, using git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' | xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/' to convert the new alloc_obj() users that had a simple GFP_KERNEL argument to just drop that argument. Note that due to the extreme simplicity of the scripting, any slightly more complex cases spread over multiple lines would not be triggered: they definitely exist, but this covers the vast bulk of the cases, and the resulting diff is also then easier to check automatically. For the same reason the 'flex' versions will be done as a separate conversion. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2026-02-21treewide: Replace kmalloc with kmalloc_obj for non-scalar typesKees Cook10-25/+24
This is the result of running the Coccinelle script from scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to avoid scalar types (which need careful case-by-case checking), and instead replace kmalloc-family calls that allocate struct or union object instances: Single allocations: kmalloc(sizeof(TYPE), ...) are replaced with: kmalloc_obj(TYPE, ...) Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...) are replaced with: kmalloc_objs(TYPE, COUNT, ...) Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...) are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...) (where TYPE may also be *VAR) The resulting allocations no longer return "void *", instead returning "TYPE *". Signed-off-by: Kees Cook <kees@kernel.org>
2026-02-18apparmor: fix signedness bug in unpack_tags()Massimiliano Pellizzer1-1/+1
Smatch static checker warning: security/apparmor/policy_unpack.c:966 unpack_pdb() warn: unsigned 'unpack_tags(e, &pdb->tags, info)' is never less than zero. unpack_tags() is declared with return type size_t (unsigned) but returns negative errno values on failure. The caller in unpack_pdb() tests the return with `< 0`, which is always false for an unsigned type, making error handling dead code. Malformed tag data would be silently accepted instead of causing a load failure. Change return type of unpack_tags() from size_t to int to match the functions's actual semantic. Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Massimiliano Pellizzer <mpellizzer.dev@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-03apparmor: fix cast in format string DEBUG statementJohn Johansen1-1/+1
if debugging is enabled the DEBUG statement will fail do to a bad fat fingered cast. Fixes: 102ada7ca37ed ("apparmor: fix fmt string type error in process_strs_entry") Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-02apparmor: fix aa_label to return state from compount and component matchJohn Johansen1-6/+6
aa-label_match is not correctly returning the state in all cases. The only reason this didn't cause a error is that all callers currently ignore the return value. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202602020631.wXgZosyU-lkp@intel.com/ Fixes: a4c9efa4dbad6 ("apparmor: make label_match return a consistent value") Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-02apparmor: fix fmt string type error in process_strs_entryJohn Johansen1-2/+3
pointer subtraction has a type of int when using clang on hexagon, microblaze (and possibly other archs). We know the subtraction is postive so cast the expression to unsigned long to match what is in the fmt string. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202602021429.CcmWkR9K-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202602021427.PvvDjgyL-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202602021510.JPzX5zKb-lkp@intel.com/ Fixes: c140dcd1246bf ("apparmor: make str table more generic and be able to have multiple entries") Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-02apparmor: fix kernel-doc comments for inviewJohn Johansen1-2/+2
subns was renamed inview to better reflect the function of the flag. Unfortunately the kernel-doc was not properly updated in 2 places. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202602020737.vGCZFds1-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202602021427.PvvDjgyL-lkp@intel.com/ Fixes: 796c146fa6c82 ("apparmor: split xxx_in_ns into its two separate semantic use cases") Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-02apparmor: fix invalid deref of rawdata when export_binary is unsetGeorgia Garcia1-0/+9
If the export_binary parameter is disabled on runtime, profiles that were loaded before that will still have their rawdata stored in apparmorfs, with a symbolic link to the rawdata on the policy directory. When one of those profiles are replaced, the rawdata is set to NULL, but when trying to resolve the symbolic links to rawdata for that profile, it will try to dereference profile->rawdata->name when profile->rawdata is now NULL causing an oops. Fix it by checking if rawdata is set. [ 168.653080] BUG: kernel NULL pointer dereference, address: 0000000000000088 [ 168.657420] #PF: supervisor read access in kernel mode [ 168.660619] #PF: error_code(0x0000) - not-present page [ 168.663613] PGD 0 P4D 0 [ 168.665450] Oops: Oops: 0000 [#1] SMP NOPTI [ 168.667836] CPU: 1 UID: 0 PID: 1729 Comm: ls Not tainted 6.19.0-rc7+ #3 PREEMPT(voluntary) [ 168.672308] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 168.679327] RIP: 0010:rawdata_get_link_base.isra.0+0x23/0x330 [ 168.682768] Code: 90 90 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 48 89 55 d0 48 85 ff 0f 84 e3 01 00 00 <48> 83 3c 25 88 00 00 00 00 0f 84 d4 01 00 00 49 89 f6 49 89 cc e8 [ 168.689818] RSP: 0018:ffffcdcb8200fb80 EFLAGS: 00010282 [ 168.690871] RAX: ffffffffaee74ec0 RBX: 0000000000000000 RCX: ffffffffb0120158 [ 168.692251] RDX: ffffcdcb8200fbe0 RSI: ffff88c187c9fa80 RDI: ffff88c186c98a80 [ 168.693593] RBP: ffffcdcb8200fbc0 R08: 0000000000000000 R09: 0000000000000000 [ 168.694941] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88c186c98a80 [ 168.696289] R13: 00007fff005aaa20 R14: 0000000000000080 R15: ffff88c188f4fce0 [ 168.697637] FS: 0000790e81c58280(0000) GS:ffff88c20a957000(0000) knlGS:0000000000000000 [ 168.699227] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 168.700349] CR2: 0000000000000088 CR3: 000000012fd3e000 CR4: 0000000000350ef0 [ 168.701696] Call Trace: [ 168.702325] <TASK> [ 168.702995] rawdata_get_link_data+0x1c/0x30 [ 168.704145] vfs_readlink+0xd4/0x160 [ 168.705152] do_readlinkat+0x114/0x180 [ 168.706214] __x64_sys_readlink+0x1e/0x30 [ 168.708653] x64_sys_call+0x1d77/0x26b0 [ 168.709525] do_syscall_64+0x81/0x500 [ 168.710348] ? do_statx+0x72/0xb0 [ 168.711109] ? putname+0x3e/0x80 [ 168.711845] ? __x64_sys_statx+0xb7/0x100 [ 168.712711] ? x64_sys_call+0x10fc/0x26b0 [ 168.713577] ? do_syscall_64+0xbf/0x500 [ 168.714412] ? do_user_addr_fault+0x1d2/0x8d0 [ 168.715404] ? irqentry_exit+0xb2/0x740 [ 168.716359] ? exc_page_fault+0x90/0x1b0 [ 168.717307] entry_SYSCALL_64_after_hwframe+0x76/0x7e Fixes: 1180b4c757aab ("apparmor: fix dangling symlinks to policy rawdata after replacement") Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-02-01apparmor: add .kunitconfigRyota Sakamoto1-0/+5
Add .kunitconfig file to the AppArmor directory to enable easy execution of KUnit tests. AppArmor tests (CONFIG_SECURITY_APPARMOR_KUNIT_TEST) depend on CONFIG_SECURITY_APPARMOR which also depends on CONFIG_SECURITY and CONFIG_NET. Without explicitly enabling these configs in the .kunitconfig, developers will need to specify config manually. With the .kunitconfig, developers can run the tests: $ ./tools/testing/kunit/kunit.py run --kunitconfig security/apparmor Signed-off-by: Ryota Sakamoto <sakamo.ryota@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: cleanup remove unused percpu critical sections in buffer managementJohn Johansen1-5/+0
There are two unused percpu critical sections in the buffer management code. These are remanents from when a more complex hold algorithm was used. Remove them, as they serve no purpose. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: document the buffer hold, add an overflow guardJohn Johansen1-2/+26
The buffer hold is a measure of contention, but it is tracked per cpu where the lock is a globabl resource. On some systems (eg. real time) there is no guarantee that the code will be on the same cpu pre, and post spinlock acquisition, nor that the buffer will be put back to the same percpu cache when we are done with it. Because of this the hold value can move asynchronous to the buffers on the cache, meaning it is possible to underflow, and potentially in really pathelogical cases overflow. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: avoid per-cpu hold underflow in aa_get_bufferZhengmian Hu1-1/+2
When aa_get_buffer() pulls from the per-cpu list it unconditionally decrements cache->hold. If hold reaches 0 while count is still non-zero, the unsigned decrement wraps to UINT_MAX. This keeps hold non-zero for a very long time, so aa_put_buffer() never returns buffers to the global list, which can starve other CPUs and force repeated kmalloc(aa_g_path_max) allocations. Guard the decrement so hold never underflows. Fixes: ea9bae12d028 ("apparmor: cache buffers on percpu list if there is lock contention") Signed-off-by: Zhengmian Hu <huzhengmian@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: split xxx_in_ns into its two separate semantic use casesJohn Johansen5-46/+61
This patch doesn't change current functionality, it switches the two uses of the in_ns fns and macros into the two semantically different cases they are used for. xxx_in_scope for checking mediation interaction between profiles xxx_in_view to determine which profiles are visible.The scope will always be a subset of the view as profiles that can not see each other can not interact. The split can not be completely done for label_match because it has to distinct uses matching permission against label in scope, and checking if a transition to a profile is allowed. The transition to a profile can include profiles that are in view but not in scope, so retain this distinction as a parameter. While at the moment the two uses are very similar, in the future there will be additional differences. So make sure the semantics differences are present in the code. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: make label_match return a consistent valueJohn Johansen1-11/+9
compound match is inconsistent in returning a state or an integer error this is problemati if the error is ever used as a state in the state machine Fixes: f1bd904175e81 ("apparmor: add the base fns() for domain labels") Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: remove apply_modes_to_perms from label_matchJohn Johansen1-3/+0
The modes shouldn't be applied at the point of label match, it just results in them being applied multiple times. Instead they should be applied after which is already being done by all callers so it can just be dropped from label_match. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: fix fast path cache check for unix socketsJohn Johansen1-14/+21
The fast path cache check is incorrect forcing more slow path revalidations than necessary, because the unix logic check is inverted. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: fix rlimit for posix cpu timersJohn Johansen1-0/+5
Posix cpu timers requires an additional step beyond setting the rlimit. Refactor the code so its clear when what code is setting the limit and conditionally update the posix cpu timers when appropriate. Fixes: baa73d9e478ff ("posix-timers: Make them configurable") Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: refactor/cleanup cred helper fns.John Johansen1-31/+69
aa_cred_raw_label() and cred_label() now do the same things so consolidate to cred_label() Document the crit section use and constraints better and refactor __begin_current_label_crit_section() into a base fn __begin_cred_crit_section() and a wrapper that calls the base with current cred. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: fix label and profile debug macrosJohn Johansen1-1/+3
The label and profile debug macros were not correctly pasting their var args. Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: move check for aa_null file to cover all casesJohn Johansen2-6/+10
files with a dentry pointing aa_null.dentry where already rejected as part of file_inheritance. Unfortunately the check in common_file_perm() is insufficient to cover all cases causing unnecessary audit messages without the original files context. Eg. [ 442.886474] audit: type=1400 audit(1704822661.616:329): apparmor="DENIED" operation="file_inherit" class="file" namespace="root//lxd-juju-98527a-0_<var-snap-lxd-common-lxd>" profile="snap.lxd.activate" name="/apparmor/.null" pid=9525 comm="snap-exec" Further examples of this are in the logs of https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2120439 https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1952084 https://bugs.launchpad.net/snapd/+bug/2049099 These messages have no value and should not be sent to the logs. AppArmor was already filtering the out in some cases but the original patch did not catch all cases. Fix this by push the existing check down into two functions that should cover all cases. Link: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2122743 Fixes: 192ca6b55a86 ("apparmor: revalidate files during exec") Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: guard against free routines being called with a NULLJohn Johansen1-0/+6
aa_free_data() and free_attachment() don't guard against having a NULL parameter passed to them. Fix this. Reviewed-by: Ryan Lee <ryan.lee@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: return -ENOMEM in unpack_perms_table upon alloc failureRyan Lee1-2/+4
In policy_unpack.c:unpack_perms_table, the perms struct is allocated via kcalloc, with the position being reset if the allocation fails. However, the error path results in -EPROTO being retured instead of -ENOMEM. Fix this to return the correct error code. Reported-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com> Fixes: fd1b2b95a2117 ("apparmor: add the ability for policy to specify a permission table") Reviewed-by: Tyler Hicks <code@tyhicks.com> Signed-off-by: Ryan Lee <ryan.lee@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
2026-01-29apparmor: account for in_atomic removal in common_file_permRyan Lee1-3/+2
If we are not in an atomic context in common_file_perm, then we don't have to use the atomic versions, resulting in improved performance outside of atomic contexts. Signed-off-by: Ryan Lee <ryan.lee@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>