aboutsummaryrefslogtreecommitdiffstats
path: root/security
Commit message (Collapse)AuthorAgeFilesLines
* Merge tag 'apparmor-pr-2025-08-04' of ↵Linus Torvalds2025-08-0438-429/+2177
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor Pull apparmor updates from John Johansen: "This has one major feature, it pulls in a cleaned up version of af_unix mediation that Ubuntu has been carrying for years. It is placed behind a new abi to ensure that it does cause policy regressions. With pulling in the af_unix mediation there have been cleanups and some refactoring of network socket mediation. This accounts for the majority of the changes in the diff. In addition there are a few improvements providing minor code optimizations. several code cleanups, and bug fixes. Features: - improve debug printing - carry mediation check on label (optimization) - improve ability for compiler to optimize __begin_current_label_crit_section - transition for a linked list of rulesets to a vector of rulesets - don't hardcode profile signal, allow it to be set by policy - ability to mediate caps via the state machine instead of lut - Add Ubuntu af_unix mediation, put it behind new v9 abi Cleanups: - fix typos and spelling errors - cleanup kernel doc and code inconsistencies - remove redundant checks/code - remove unused variables - Use str_yes_no() helper function - mark tables static where appropriate - make all generated string array headers const char *const - refactor to doc semantics of file_perm checks - replace macro calls to network/socket fns with explicit calls - refactor/cleanup socket mediation code preparing for finer grained mediation of different network families - several updates to kernel doc comments Bug fixes: - fix incorrect profile->signal range check - idmap mount fixes - policy unpack unaligned access fixes - kfree_sensitive() where appropriate - fix oops when freeing policy - fix conflicting attachment resolution - fix exec table look-ups when stacking isn't first - fix exec auditing - mitigate userspace generating overly large xtables" * tag 'apparmor-pr-2025-08-04' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: (60 commits) apparmor: fix: oops when trying to free null ruleset apparmor: fix Regression on linux-next (next-20250721) apparmor: fix test error: WARNING in apparmor_unix_stream_connect apparmor: Remove the unused variable rules apparmor: fix: accept2 being specifie even when permission table is presnt apparmor: transition from a list of rules to a vector of rules apparmor: fix documentation mismatches in val_mask_to_str and socket functions apparmor: remove redundant perms.allow MAY_EXEC bitflag set apparmor: fix kernel doc warnings for kernel test robot apparmor: Fix unaligned memory accesses in KUnit test apparmor: Fix 8-byte alignment for initial dfa blob streams apparmor: shift uid when mediating af_unix in userns apparmor: shift ouid when mediating hard links in userns apparmor: make sure unix socket labeling is correctly updated. apparmor: fix regression in fs based unix sockets when using old abi apparmor: fix AA_DEBUG_LABEL() apparmor: fix af_unix auditing to include all address information apparmor: Remove use of the double lock apparmor: update kernel doc comments for xxx_label_crit_section apparmor: make __begin_current_label_crit_section() indicate whether put is needed ...
| * apparmor: fix: oops when trying to free null rulesetJohn Johansen2025-08-041-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | profile allocation is wrongly setting the number of entries on the rules vector before any ruleset is assigned. If profile allocation fails between ruleset allocation and assigning the first ruleset, free_ruleset() will be called with a null pointer resulting in an oops. [ 107.350226] kernel BUG at mm/slub.c:545! [ 107.350912] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI [ 107.351447] CPU: 1 UID: 0 PID: 27 Comm: ksoftirqd/1 Not tainted 6.14.6-hwe-rlee287-dev+ #5 [ 107.353279] Hardware name:[ 107.350218] -QE-----------[ cutMU here ]--------- Ub--- [ 107.3502untu26] kernel BUG a 24t mm/slub.c:545.!04 P [ 107.350912]C ( Oops: invalid oi4pcode: 0000 [#1]40 PREEMPT SMP NOPFXTI + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 107.356054] RIP: 0010:__slab_free+0x152/0x340 [ 107.356444] Code: 00 4c 89 ff e8 0f ac df 00 48 8b 14 24 48 8b 4c 24 20 48 89 44 24 08 48 8b 03 48 c1 e8 09 83 e0 01 88 44 24 13 e9 71 ff ff ff <0f> 0b 41 f7 44 24 08 87 04 00 00 75 b2 eb a8 41 f7 44 24 08 87 04 [ 107.357856] RSP: 0018:ffffad4a800fbbb0 EFLAGS: 00010246 [ 107.358937] RAX: ffff97ebc2a88e70 RBX: ffffd759400aa200 RCX: 0000000000800074 [ 107.359976] RDX: ffff97ebc2a88e60 RSI: ffffd759400aa200 RDI: ffffad4a800fbc20 [ 107.360600] RBP: ffffad4a800fbc50 R08: 0000000000000001 R09: ffffffff86f02cf2 [ 107.361254] R10: 0000000000000000 R11: 0000000000000000 R12: ffff97ecc0049400 [ 107.361934] R13: ffff97ebc2a88e60 R14: ffff97ecc0049400 R15: 0000000000000000 [ 107.362597] FS: 0000000000000000(0000) GS:ffff97ecfb200000(0000) knlGS:0000000000000000 [ 107.363332] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 107.363784] CR2: 000061c9545ac000 CR3: 0000000047aa6000 CR4: 0000000000750ef0 [ 107.364331] PKRU: 55555554 [ 107.364545] Call Trace: [ 107.364761] <TASK> [ 107.364931] ? local_clock+0x15/0x30 [ 107.365219] ? srso_alias_return_thunk+0x5/0xfbef5 [ 107.365593] ? kfree_sensitive+0x32/0x70 [ 107.365900] kfree+0x29d/0x3a0 [ 107.366144] ? srso_alias_return_thunk+0x5/0xfbef5 [ 107.366510] ? local_clock_noinstr+0xe/0xd0 [ 107.366841] ? srso_alias_return_thunk+0x5/0xfbef5 [ 107.367209] kfree_sensitive+0x32/0x70 [ 107.367502] aa_free_profile.part.0+0xa2/0x400 [ 107.367850] ? rcu_do_batch+0x1e6/0x5e0 [ 107.368148] aa_free_profile+0x23/0x60 [ 107.368438] label_free_switch+0x4c/0x80 [ 107.368751] label_free_rcu+0x1c/0x50 [ 107.369038] rcu_do_batch+0x1e8/0x5e0 [ 107.369324] ? rcu_do_batch+0x157/0x5e0 [ 107.369626] rcu_core+0x1b0/0x2f0 [ 107.369888] rcu_core_si+0xe/0x20 [ 107.370156] handle_softirqs+0x9b/0x3d0 [ 107.370460] ? smpboot_thread_fn+0x26/0x210 [ 107.370790] run_ksoftirqd+0x3a/0x70 [ 107.371070] smpboot_thread_fn+0xf9/0x210 [ 107.371383] ? __pfx_smpboot_thread_fn+0x10/0x10 [ 107.371746] kthread+0x10d/0x280 [ 107.372010] ? __pfx_kthread+0x10/0x10 [ 107.372310] ret_from_fork+0x44/0x70 [ 107.372655] ? __pfx_kthread+0x10/0x10 [ 107.372974] ret_from_fork_asm+0x1a/0x30 [ 107.373316] </TASK> [ 107.373505] Modules linked in: af_packet_diag mptcp_diag tcp_diag udp_diag raw_diag inet_diag snd_seq_dummy snd_hrtimer snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore qrtr binfmt_misc intel_rapl_msr intel_rapl_common kvm_amd ccp kvm irqbypass polyval_clmulni polyval_generic ghash_clmulni_intel sha256_ssse3 sha1_ssse3 aesni_intel crypto_simd cryptd i2c_piix4 i2c_smbus input_leds joydev sch_fq_codel msr parport_pc ppdev lp parport efi_pstore nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vsock vmw_vmci dmi_sysfs qemu_fw_cfg ip_tables x_tables autofs4 hid_generic usbhid hid psmouse serio_raw floppy bochs pata_acpi [ 107.379086] ---[ end trace 0000000000000000 ]--- Don't set the count until a ruleset is actually allocated and guard against free_ruleset() being called with a null pointer. Reported-by: Ryan Lee <[email protected]> Fixes: 217af7e2f4de ("apparmor: refactor profile rules and attachments") Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix Regression on linux-next (next-20250721)John Johansen2025-07-301-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sk lock initialization was incorrectly removed, from apparmor_file_alloc_security() while testing changes to changes to apparmor_sk_alloc_security() resulting in the following regression. [ 48.056654] INFO: trying to register non-static key. [ 48.057480] The code is fine but needs lockdep annotation, or maybe [ 48.058416] you didn't initialize this object before use? [ 48.059209] turning off the locking correctness validator. [ 48.060040] CPU: 0 UID: 0 PID: 648 Comm: chronyd Not tainted 6.16.0-rc7-test-next-20250721-11410-g1ee809985e11-dirty #577 NONE [ 48.060049] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 48.060055] Call Trace: [ 48.060059] <TASK> [ 48.060063] dump_stack_lvl (lib/dump_stack.c:122) [ 48.060075] register_lock_class (kernel/locking/lockdep.c:988 kernel/locking/lockdep.c:1302) [ 48.060084] ? path_name (security/apparmor/file.c:159) [ 48.060093] __lock_acquire (kernel/locking/lockdep.c:5116) [ 48.060103] lock_acquire (kernel/locking/lockdep.c:473 (discriminator 4) kernel/locking/lockdep.c:5873 (discriminator 4) kernel/locking/lockdep.c:5828 (discriminator 4)) [ 48.060109] ? update_file_ctx (security/apparmor/file.c:464) [ 48.060115] ? __pfx_profile_path_perm (security/apparmor/file.c:247) [ 48.060121] _raw_spin_lock (include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154) [ 48.060130] ? update_file_ctx (security/apparmor/file.c:464) [ 48.060134] update_file_ctx (security/apparmor/file.c:464) [ 48.060140] aa_file_perm (security/apparmor/file.c:532 (discriminator 1) security/apparmor/file.c:642 (discriminator 1)) [ 48.060147] ? __pfx_aa_file_perm (security/apparmor/file.c:607) [ 48.060152] ? do_mmap (mm/mmap.c:558) [ 48.060160] ? __pfx_userfaultfd_unmap_complete (fs/userfaultfd.c:841) [ 48.060170] ? __lock_acquire (kernel/locking/lockdep.c:4677 (discriminator 1) kernel/locking/lockdep.c:5194 (discriminator 1)) [ 48.060176] ? common_file_perm (security/apparmor/lsm.c:535 (discriminator 1)) [ 48.060185] security_mmap_file (security/security.c:3012 (discriminator 2)) [ 48.060192] vm_mmap_pgoff (mm/util.c:574 (discriminator 1)) [ 48.060200] ? find_held_lock (kernel/locking/lockdep.c:5353 (discriminator 1)) [ 48.060206] ? __pfx_vm_mmap_pgoff (mm/util.c:568) [ 48.060212] ? lock_release (kernel/locking/lockdep.c:5539 kernel/locking/lockdep.c:5892 kernel/locking/lockdep.c:5878) [ 48.060219] ? __fget_files (arch/x86/include/asm/preempt.h:85 (discriminator 13) include/linux/rcupdate.h:100 (discriminator 13) include/linux/rcupdate.h:873 (discriminator 13) fs/file.c:1072 (discriminator 13)) [ 48.060229] ksys_mmap_pgoff (mm/mmap.c:604) [ 48.060239] do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1)) [ 48.060248] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) [ 48.060254] RIP: 0033:0x7fb6920e30a2 [ 48.060265] Code: 08 00 04 00 00 eb e2 90 41 f7 c1 ff 0f 00 00 75 27 55 89 cd 53 48 89 fb 48 85 ff 74 33 41 89 ea 48 89 df b8 09 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 5e 5b 5d c3 0f 1f 00 c7 05 e6 41 01 00 16 00 All code ======== 0: 08 00 or %al,(%rax) 2: 04 00 add $0x0,%al 4: 00 eb add %ch,%bl 6: e2 90 loop 0xffffffffffffff98 8: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d f: 75 27 jne 0x38 11: 55 push %rbp 12: 89 cd mov %ecx,%ebp 14: 53 push %rbx 15: 48 89 fb mov %rdi,%rbx 18: 48 85 ff test %rdi,%rdi 1b: 74 33 je 0x50 1d: 41 89 ea mov %ebp,%r10d 20: 48 89 df mov %rbx,%rdi 23: b8 09 00 00 00 mov $0x9,%eax 28: 0f 05 syscall 2a:* 48 3d 00 f0 ff ff cmp $0xfffffffffffff000,%rax <-- trapping instruction 30: 77 5e ja 0x90 32: 5b pop %rbx 33: 5d pop %rbp 34: c3 ret 35: 0f 1f 00 nopl (%rax) 38: c7 .byte 0xc7 39: 05 e6 41 01 00 add $0x141e6,%eax 3e: 16 (bad) ... Code starting with the faulting instruction =========================================== 0: 48 3d 00 f0 ff ff cmp $0xfffffffffffff000,%rax 6: 77 5e ja 0x66 8: 5b pop %rbx 9: 5d pop %rbp a: c3 ret b: 0f 1f 00 nopl (%rax) e: c7 .byte 0xc7 f: 05 e6 41 01 00 add $0x141e6,%eax 14: 16 (bad) ... [ 48.060270] RSP: 002b:00007ffd2c0d3528 EFLAGS: 00000206 ORIG_RAX: 0000000000000009 [ 48.060279] RAX: ffffffffffffffda RBX: 00007fb691fc8000 RCX: 00007fb6920e30a2 [ 48.060283] RDX: 0000000000000005 RSI: 000000000007d000 RDI: 00007fb691fc8000 [ 48.060287] RBP: 0000000000000812 R08: 0000000000000003 R09: 0000000000011000 [ 48.060290] R10: 0000000000000812 R11: 0000000000000206 R12: 00007ffd2c0d3578 [ 48.060293] R13: 00007fb6920b6160 R14: 00007ffd2c0d39f0 R15: 00000fffa581a6a8 Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.") Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix test error: WARNING in apparmor_unix_stream_connectJohn Johansen2025-07-301-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.") added the use of security_sk_alloc() which ensures the sk label is initialized. This means that the AA_BUG in apparmor_unix_stream_connect() is no longer correct, because while the sk is still not being initialized by going through post_create, it is now initialize in sk_alloc(). Remove the now invalid check. Reported-by: [email protected] Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.") Signed-off-by: John Johansen <[email protected]>
| * apparmor: Remove the unused variable rulesJiapeng Chong2025-07-301-2/+0
| | | | | | | | | | | | | | | | | | | | | | Variable rules is not effectively used, so delete it. security/apparmor/lsm.c:182:23: warning: variable ‘rules’ set but not used. Reported-by: Abaci Robot <[email protected]> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=22942 Signed-off-by: Jiapeng Chong <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix: accept2 being specifie even when permission table is presntJohn Johansen2025-07-201-1/+2
| | | | | | | | | | | | | | | | | | | | The transition to the perms32 permission table dropped the need for the accept2 table as permissions. However accept2 can be used for flags and may be present even when the perms32 table is present. So instead of checking on version, check whether the table is present. Fixes: 2e12c5f06017 ("apparmor: add additional flags to extended permission.") Signed-off-by: John Johansen <[email protected]>
| * apparmor: transition from a list of rules to a vector of rulesJohn Johansen2025-07-2015-113/+85
| | | | | | | | | | | | | | | | | | The set of rules on a profile is not dynamically extended, instead if a new ruleset is needed a new version of the profile is created. This allows us to use a vector of rules instead of a list, slightly reducing memory usage and simplifying the code. Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix documentation mismatches in val_mask_to_str and socket functionsPeng Jiang2025-07-202-3/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes kernel-doc warnings: 1. val_mask_to_str: - Added missing descriptions for `size` and `table` parameters. - Removed outdated str_size and chrs references. 2. Socket Functions: - Makes non-null requirements clear for socket/address args. - Standardizes return values per kernel conventions. - Adds Unix domain socket protocol details. These changes silence doc validation warnings and improve accuracy for AppArmor LSM docs. Signed-off-by: Peng Jiang <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: remove redundant perms.allow MAY_EXEC bitflag setRyan Lee2025-07-201-3/+1
| | | | | | | | | | | | | | | | | | | | This section of profile_transition that occurs after x_to_label only happens if perms.allow already has the MAY_EXEC bit set, so we don't need to set it again. Fixes: 16916b17b4f8 ("apparmor: force auditing of conflicting attachment execs from confined") Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix kernel doc warnings for kernel test robotJohn Johansen2025-07-202-4/+10
| | | | | | | | | | | | | | | | | | | | | | | | Fix kernel doc warnings for the functions - apparmor_socket_bind - apparmor_unix_may_send - apparmor_unix_stream_connect - val_mask_to_str Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ Signed-off-by: John Johansen <[email protected]>
| * apparmor: Fix unaligned memory accesses in KUnit testHelge Deller2025-07-201-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The testcase triggers some unnecessary unaligned memory accesses on the parisc architecture: Kernel: unaligned access to 0x12f28e27 in policy_unpack_test_init+0x180/0x374 (iir 0x0cdc1280) Kernel: unaligned access to 0x12f28e67 in policy_unpack_test_init+0x270/0x374 (iir 0x64dc00ce) Use the existing helper functions put_unaligned_le32() and put_unaligned_le16() to avoid such warnings on architectures which prefer aligned memory accesses. Signed-off-by: Helge Deller <[email protected]> Fixes: 98c0cc48e27e ("apparmor: fix policy_unpack_test on big endian systems") Signed-off-by: John Johansen <[email protected]>
| * apparmor: Fix 8-byte alignment for initial dfa blob streamsHelge Deller2025-07-201-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The dfa blob stream for the aa_dfa_unpack() function is expected to be aligned on a 8 byte boundary. The static nulldfa_src[] and stacksplitdfa_src[] arrays store the initial apparmor dfa blob streams, but since they are declared as an array-of-chars the compiler and linker will only ensure a "char" (1-byte) alignment. Add an __aligned(8) annotation to the arrays to tell the linker to always align them on a 8-byte boundary. This avoids runtime warnings at startup on alignment-sensitive platforms like parisc such as: Kernel: unaligned access to 0x7f2a584a in aa_dfa_unpack+0x124/0x788 (iir 0xca0109f) Kernel: unaligned access to 0x7f2a584e in aa_dfa_unpack+0x210/0x788 (iir 0xca8109c) Kernel: unaligned access to 0x7f2a586a in aa_dfa_unpack+0x278/0x788 (iir 0xcb01090) Signed-off-by: Helge Deller <[email protected]> Cc: [email protected] Fixes: 98b824ff8984 ("apparmor: refcount the pdb") Signed-off-by: John Johansen <[email protected]>
| * apparmor: shift uid when mediating af_unix in usernsGabriel Totev2025-07-201-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid unshifted ouids for socket file operations as observed when using AppArmor profiles in unprivileged containers with LXD or Incus. For example, root inside container and uid 1000000 outside, with `owner /root/sock rw,` profile entry for nc: /root$ nc -lkU sock & nc -U sock ==> dmesg apparmor="DENIED" operation="connect" class="file" namespace="root//lxd-podia_<var-snap-lxd-common-lxd>" profile="sockit" name="/root/sock" pid=3924 comm="nc" requested_mask="wr" denied_mask="wr" fsuid=1000000 ouid=0 [<== should be 1000000] Fix by performing uid mapping as per common_perm_cond() in lsm.c Signed-off-by: Gabriel Totev <[email protected]> Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <[email protected]>
| * apparmor: shift ouid when mediating hard links in usernsGabriel Totev2025-07-201-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using AppArmor profiles inside an unprivileged container, the link operation observes an unshifted ouid. (tested with LXD and Incus) For example, root inside container and uid 1000000 outside, with `owner /root/link l,` profile entry for ln: /root$ touch chain && ln chain link ==> dmesg apparmor="DENIED" operation="link" class="file" namespace="root//lxd-feet_<var-snap-lxd-common-lxd>" profile="linkit" name="/root/link" pid=1655 comm="ln" requested_mask="l" denied_mask="l" fsuid=1000000 ouid=0 [<== should be 1000000] target="/root/chain" Fix by mapping inode uid of old_dentry in aa_path_link() rather than using it directly, similarly to how it's mapped in __file_path_perm() later in the file. Signed-off-by: Gabriel Totev <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: make sure unix socket labeling is correctly updated.John Johansen2025-07-206-62/+231
| | | | | | | | | | | | | | | | | | When a unix socket is passed into a different confinement domain make sure its cached mediation labeling is updated to correctly reflect which domains are using the socket. Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix regression in fs based unix sockets when using old abiJohn Johansen2025-07-162-51/+71
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Policy loaded using abi 7 socket mediation was not being applied correctly in all cases. In some cases with fs based unix sockets a subset of permissions where allowed when they should have been denied. This was happening because the check for if the socket was an fs based unix socket came before the abi check. But the abi check is where the correct path is selected, so having the fs unix socket check occur early would cause the wrong code path to be used. Fix this by pushing the fs unix to be done after the abi check. Fixes: dcd7a559411e ("apparmor: gate make fine grained unix mediation behind v9 abi") Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix AA_DEBUG_LABEL()John Johansen2025-07-161-1/+1
| | | | | | | | | | | | | | | | AA_DEBUG_LABEL() was not specifying it vargs, which is needed so it can output debug parameters. Fixes: 71e6cff3e0dd ("apparmor: Improve debug print infrastructure") Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix af_unix auditing to include all address informationJohn Johansen2025-07-163-10/+18
| | | | | | | | | | | | | | | | | | The auditing of addresses currently doesn't include the source address and mixes source and foreign/peer under the same audit name. Fix this so source is always addr, and the foreign/peer is peer_addr. Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <[email protected]>
| * apparmor: Remove use of the double lockJohn Johansen2025-07-165-102/+104
| | | | | | | | | | | | | | | | | | The use of the double lock is not necessary and problematic. Instead pull the bits that need locks into their own sections and grab the needed references. Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <[email protected]>
| * apparmor: update kernel doc comments for xxx_label_crit_sectionJohn Johansen2025-07-161-0/+8
| | | | | | | | | | | | | | | | Add a kernel doc header for __end_current_label_crit_section(), and update the header for __begin_current_label_crit_section(). Fixes: b42ecc5f58ef ("apparmor: make __begin_current_label_crit_section() indicate whether put is needed") Signed-off-by: John Johansen <[email protected]>
| * apparmor: make __begin_current_label_crit_section() indicate whether put is ↵Mateusz Guzik2025-07-163-41/+67
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | needed Same as aa_get_newest_cred_label_condref(). This avoids a bunch of work overall and allows the compiler to note when no clean up is necessary, allowing for tail calls. This in particular happens in apparmor_file_permission(), which manages to tail call aa_file_perm() 105 bytes in (vs a regular call 112 bytes in followed by branches to figure out if clean up is needed). Signed-off-by: Mateusz Guzik <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * Revert "apparmor: use SHA-256 library API instead of crypto_shash API"John Johansen2025-07-162-13/+75
| | | | | | | | | | | | | | | | | | This reverts commit e9ed1eb8f6217e53843d82ecf2d50f8d1a93e77c. Eric has requested that this patch be taken through the libcrypto-next tree, instead. Signed-off-by: John Johansen <[email protected]>
| * apparmor: mitigate parser generating large xtablesJohn Johansen2025-07-163-6/+45
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some versions of the parser are generating an xtable transition per state in the state machine, even when the state machine isn't using the transition table. The parser bug is triggered by commit 2e12c5f06017 ("apparmor: add additional flags to extended permission.") In addition to fixing this in userspace, mitigate this in the kernel as part of the policy verification checks by detecting this situation and adjusting to what is actually used, or if not used at all freeing it, so we are not wasting unneeded memory on policy. Fixes: 2e12c5f06017 ("apparmor: add additional flags to extended permission.") Signed-off-by: John Johansen <[email protected]>
| * apparmor: Document that label must be last member in struct aa_profileJohn Johansen2025-05-261-1/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The label struct is variable length. While its use in struct aa_profile is fixed length at 2 entries the variable length member needs to be the last member in the structure. The code already does this but the comment has it in the wrong location. Also add a comment to ensure it stays at the end of the structure. While we are at it, update the documentation for other profile members as well. Signed-off-by: John Johansen <[email protected]>
| * apparmor: make debug_values_table staticJohn Johansen2025-05-261-1/+1
| | | | | | | | | | | | | | The debug_values_table is only referenced from lib.c so it should be static. Signed-off-by: John Johansen <[email protected]>
| * apparmor: force auditing of conflicting attachment execs from confinedRyan Lee2025-05-261-0/+9
| | | | | | | | | | | | | | | | | | Conflicting attachment paths are an error state that result in the binary in question executing under an unexpected ix/ux fallback. As such, it should be audited to record the occurrence of conflicting attachments. Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: include conflicting attachment info for confined ix/ux fallbackRyan Lee2025-05-261-2/+33
| | | | | | | | | | | | | | | | | | | | Instead of silently overwriting the conflicting profile attachment string, include that information in the ix/ux fallback string that gets set as info instead. Also add a warning print if some other info is set that would be overwritten by the ix/ux fallback string or by the profile not found error. Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: move the "conflicting profile attachments" infostr to a const ↵Ryan Lee2025-05-261-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | declaration Instead of having a literal, making this a constant will allow for (hacky) detection of conflicting profile attachments from inspection of the info pointer. This is used in the next patch to augment the information provided through domain.c:x_to_label for ix/ux fallback. Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: force audit on unconfined exec if info is set by find_attachRyan Lee2025-05-261-0/+16
| | | | | | | | | | | | | | | | | | | | find_attach may set info if something unusual happens during that process (currently only used to signal conflicting attachments, but this could be expanded in the future). This is information that should be propagated to userspace via an audit message. Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: make all generated string array headers const char *constRyan Lee2025-05-261-2/+2
| | | | | | | | | | | | | | | | | | | | address_family_names and sock_type_names were created as const char *a[], which declares them as (non-const) pointers to const chars. Since the pointers themselves would not be changed, they should be generated as const char *const a[]. Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix loop detection used in conflicting attachment resolutionRyan Lee2025-05-262-15/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Conflicting attachment resolution is based on the number of states traversed to reach an accepting state in the attachment DFA, accounting for DFA loops traversed during the matching process. However, the loop counting logic had multiple bugs: - The inc_wb_pos macro increments both position and length, but length is supposed to saturate upon hitting buffer capacity, instead of wrapping around. - If no revisited state is found when traversing the history, is_loop would still return true, as if there was a loop found the length of the history buffer, instead of returning false and signalling that no loop was found. As a result, the adjustment step of aa_dfa_leftmatch would sometimes produce negative counts with loop- free DFAs that traversed enough states. - The iteration in the is_loop for loop is supposed to stop before i = wb->len, so the conditional should be < instead of <=. This patch fixes the above bugs as well as the following nits: - The count and size fields in struct match_workbuf were not used, so they can be removed. - The history buffer in match_workbuf semantically stores aa_state_t and not unsigned ints, even if aa_state_t is currently unsigned int. - The local variables in is_loop are counters, and thus should be unsigned ints instead of aa_state_t's. Fixes: 21f606610502 ("apparmor: improve overlapping domain attachment resolution") Signed-off-by: Ryan Lee <[email protected]> Co-developed-by: John Johansen <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: ensure WB_HISTORY_SIZE value is a power of 2Ryan Lee2025-05-182-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | WB_HISTORY_SIZE was defined to be a value not a power of 2, despite a comment in the declaration of struct match_workbuf stating it is and a modular arithmetic usage in the inc_wb_pos macro assuming that it is. Bump WB_HISTORY_SIZE's value up to 32 and add a BUILD_BUG_ON_NOT_POWER_OF_2 line to ensure that any future changes to the value of WB_HISTORY_SIZE respect this requirement. Fixes: 136db994852a ("apparmor: increase left match history buffer size") Signed-off-by: Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix some kernel-doc issues in header filesRandy Dunlap2025-05-175-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix kernel-doc warnings in apparmor header files as reported by scripts/kernel-doc: cred.h:128: warning: expecting prototype for end_label_crit_section(). Prototype was for end_current_label_crit_section() instead file.h:108: warning: expecting prototype for aa_map_file_perms(). Prototype was for aa_map_file_to_perms() instead lib.h:159: warning: Function parameter or struct member 'hname' not described in 'basename' lib.h:159: warning: Excess function parameter 'name' description in 'basename' match.h:21: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst * The format used for transition tables is based on the GNU flex table perms.h:109: warning: Function parameter or struct member 'accum' not described in 'aa_perms_accum_raw' perms.h:109: warning: Function parameter or struct member 'addend' not described in 'aa_perms_accum_raw' perms.h:136: warning: Function parameter or struct member 'accum' not described in 'aa_perms_accum' perms.h:136: warning: Function parameter or struct member 'addend' not described in 'aa_perms_accum' Signed-off-by: Randy Dunlap <[email protected]> Reviewed-by: Ryan Lee <[email protected]> Cc: John Johansen <[email protected]> Cc: John Johansen <[email protected]> Cc: [email protected] Cc: [email protected] Cc: Paul Moore <[email protected]> Cc: James Morris <[email protected]> Cc: "Serge E. Hallyn" <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: Fix incorrect profile->signal range checkColin Ian King2025-05-171-1/+1
| | | | | | | | | | | | | | | | | | | | The check on profile->signal is always false, the value can never be less than 1 *and* greater than MAXMAPPED_SIG. Fix this by replacing the logical operator && with ||. Fixes: 84c455decf27 ("apparmor: add support for profiles to define the kill signal") Signed-off-by: Colin Ian King <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: use SHA-256 library API instead of crypto_shash APIEric Biggers2025-05-172-75/+13
| | | | | | | | | | | | | | | | | | This user of SHA-256 does not support any other algorithm, so the crypto_shash abstraction provides no value. Just use the SHA-256 library API instead, which is much simpler and easier to use. Signed-off-by: Eric Biggers <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * security/apparmor: use kfree_sensitive() in unpack_secmark()Zilin Guan2025-05-171-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The unpack_secmark() function currently uses kfree() to release memory allocated for secmark structures and their labels. However, if a failure occurs after partially parsing secmark, sensitive data may remain in memory, posing a security risk. To mitigate this, replace kfree() with kfree_sensitive() for freeing secmark structures and their labels, aligning with the approach used in free_ruleset(). I am submitting this as an RFC to seek freedback on whether this change is appropriate and aligns with the subsystem's expectations. If confirmed to be helpful, I will send a formal patch. Signed-off-by: Zilin Guan <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: Remove unused variable 'sock' in __file_sock_perm()Nathan Chancellor2025-02-101-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS is disabled, there is a warning that sock is unused: security/apparmor/file.c: In function '__file_sock_perm': security/apparmor/file.c:544:24: warning: unused variable 'sock' [-Wunused-variable] 544 | struct socket *sock = (struct socket *) file->private_data; | ^~~~ sock was moved into aa_sock_file_perm(), where the same check is present, so remove sock and the assertion from __file_sock_perm() to fix the warning. Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ Signed-off-by: Nathan Chancellor <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: use the condition in AA_BUG_FMT even with debug disabledMateusz Guzik2025-02-101-1/+5
| | | | | | | | | | | | | | | | | | | | | | This follows the established practice and fixes a build failure for me: security/apparmor/file.c: In function ‘__file_sock_perm’: security/apparmor/file.c:544:24: error: unused variable ‘sock’ [-Werror=unused-variable] 544 | struct socket *sock = (struct socket *) file->private_data; | ^~~~ Signed-off-by: Mateusz Guzik <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix typos and spelling errorsTanya Agarwal2025-02-105-9/+9
| | | | | | | | | | | | | | | | | | | | | | Fix typos and spelling errors in apparmor module comments that were identified using the codespell tool. No functional changes - documentation only. Signed-off-by: Tanya Agarwal <[email protected]> Reviewed-by: Mimi Zohar <[email protected]> Ryan Lee <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: Modify mismatched function nameJiapeng Chong2025-02-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | No functional modification involved. security/apparmor/lib.c:93: warning: expecting prototype for aa_mask_to_str(). Prototype was for val_mask_to_str() instead. Reported-by: Abaci Robot <[email protected]> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=13606 Signed-off-by: Jiapeng Chong <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: Modify mismatched function nameJiapeng Chong2025-02-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | No functional modification involved. security/apparmor/file.c:184: warning: expecting prototype for aa_lookup_fperms(). Prototype was for aa_lookup_condperms() instead. Reported-by: Abaci Robot <[email protected]> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=13605 Signed-off-by: Jiapeng Chong <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: Fix checking address of an array in accum_label_info()Nathan Chancellor2025-02-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | clang warns: security/apparmor/label.c:206:15: error: address of array 'new->vec' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] 206 | AA_BUG(!new->vec); | ~~~~~~^~~ The address of this array can never be NULL because it is not at the beginning of a structure. Convert the assertion to check that the new pointer is not NULL. Fixes: de4754c801f4 ("apparmor: carry mediation check on label") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ Signed-off-by: Nathan Chancellor <[email protected]> Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix dbus permission queries to v9 ABIJohn Johansen2025-01-181-0/+8
| | | | | | | | | | | | | | | | | | | | | | dbus permission queries need to be synced with fine grained unix mediation to avoid potential policy regressions. To ensure that dbus queries don't result in a case where fine grained unix mediation is not being applied but dbus mediation is check the loaded policy support ABI and abort the query if policy doesn't support the v9 ABI. Signed-off-by: John Johansen <[email protected]>
| * apparmor: gate make fine grained unix mediation behind v9 abiJohn Johansen2025-01-184-10/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fine grained unix mediation in Ubuntu used ABI v7, and policy using this has propogated onto systems where fine grained unix mediation was not supported. The userspace policy compiler supports downgrading policy so the policy could be shared without changes. Unfortunately this had the side effect that policy was not updated for the none Ubuntu systems and enabling fine grained unix mediation on those systems means that a new kernel can break a system with existing policy that worked with the previous kernel. With fine grained af_unix mediation this regression can easily break the system causing boot to fail, as it affect unix socket files, non-file based unix sockets, and dbus communication. To aoid this regression move fine grained af_unix mediation behind a new abi. This means that the system's userspace and policy must be updated to support the new policy before it takes affect and dropping a new kernel on existing system will not result in a regression. The abi bump is done in such a way as existing policy can be activated on the system by changing the policy abi declaration and existing unix policy rules will apply. Policy then only needs to be incrementally updated, can even be backported to existing Ubuntu policy. Signed-off-by: John Johansen <[email protected]>
| * apparmor: add fine grained af_unix mediationJohn Johansen2025-01-1812-58/+1063
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Extend af_unix mediation to support fine grained controls based on the type (abstract, anonymous, fs), the address, and the labeling on the socket. This allows for using socket addresses to label and the socket and control which subjects can communicate. The unix rule format follows standard apparmor rules except that fs based unix sockets can be mediated by existing file rules. None fs unix sockets can be mediated by a unix socket rule. Where The address of an abstract unix domain socket begins with the @ character, similar to how they are reported (as paths) by netstat -x. The address then follows and may contain pattern matching and any characters including the null character. In apparmor null characters must be specified by using an escape sequence \000 or \x00. The pattern matching is the same as is used by file path matching so * will not match / even though it has no special meaning with in an abstract socket name. Eg. allow unix addr=@*, Autobound unix domain sockets have a unix sun_path assigned to them by the kernel, as such specifying a policy based address is not possible. The autobinding of sockets can be controlled by specifying the special auto keyword. Eg. allow unix addr=auto, To indicate that the rule only applies to auto binding of unix domain sockets. It is important to note this only applies to the bind permission as once the socket is bound to an address it is indistinguishable from a socket that have an addr bound with a specified name. When the auto keyword is used with other permissions or as part of a peer addr it will be replaced with a pattern that can match an autobound socket. Eg. For some kernels allow unix rw addr=auto, It is important to note, this pattern may match abstract sockets that were not autobound but have an addr that fits what is generated by the kernel when autobinding a socket. Anonymous unix domain sockets have no sun_path associated with the socket address, however it can be specified with the special none keyword to indicate the rule only applies to anonymous unix domain sockets. Eg. allow unix addr=none, If the address component of a rule is not specified then the rule applies to autobind, abstract and anonymous sockets. The label on the socket can be compared using the standard label= rule conditional. Eg. allow unix addr=@foo peer=(label=bar), see man apparmor.d for full syntax description. Signed-off-by: John Johansen <[email protected]>
| * apparmor: in preparation for finer networking rules rework match_protJohn Johansen2025-01-182-14/+75
| | | | | | | | | | | | | | | | Rework match_prot into a common fn that can be shared by all the networking rules. This will provide compatibility with current socket mediation, via the early bailout permission encoding. Signed-off-by: John Johansen <[email protected]>
| * apparmor: lift kernel socket check out of critical sectionJohn Johansen2025-01-181-1/+5
| | | | | | | | | | | | | | | | | | | | | | There is no need for the kern check to be in the critical section, it only complicates the code and slows down the case where the socket is being created by the kernel. Lifting it out will also allow socket_create to share common template code, with other socket_permission checks. Signed-off-by: John Johansen <[email protected]>
| * apparmor: remove af_select macroJohn Johansen2025-01-182-36/+9
| | | | | | | | | | | | | | The af_select macro just adds a layer of unnecessary abstraction that makes following what the code is doing harder. Signed-off-by: John Johansen <[email protected]>
| * apparmor: add ability to mediate caps with policy state machineJohn Johansen2025-01-183-6/+62
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently the caps encoding is very limited and can't be used with conditionals. Allow capabilities to be mediated by the state machine. This will allow us to add conditionals to capabilities that aren't possible with the current encoding. This patch only adds support for using the state machine and retains the old encoding lookup as part of the runtime mediation code to support older policy abis. A follow on patch will move backwards compatibility to a mapping function done at policy load time. Signed-off-by: John Johansen <[email protected]>
| * apparmor: fix x_table_lookup when stacking is not the first entryJohn Johansen2025-01-181-23/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | x_table_lookup currently does stacking during label_parse() if the target specifies a stack but its only caller ensures that it will never be used with stacking. Refactor to slightly simplify the code in x_to_label(), this also fixes a long standing problem where x_to_labels check on stacking is only on the first element to the table option list, instead of the element that is found and used. Signed-off-by: John Johansen <[email protected]>