apparmor_secmark_init() parses a configured secmark label to obtain its
secid. aa_label_strn_parse() returns a refcounted label, but the success
path kept that reference after copying the secid.
Fixes: ab9f211508 ("apparmor: Allow filtering based on secmark policy")
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa_getprocattr() allocates the output string before rendering the label
into it. If the second aa_label_snxprint() call fails, the function
returned without freeing that allocation.
Free and clear the output pointer on the uncommon formatting failure path
before dropping the namespace reference.
Fixes: 76a1d263ab ("apparmor: switch getprocattr to using label_print fns()")
Reviewed-by: Tyler Hicks <code@thicks.com>
Reviewed-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: John Johansen <john.johansen@canonical.com>
There is no need for a goto a label immediately following the
conditional block when the jump is the last statement in the block.
Fixes: 7306c41672 ("apparmor: release exe file resources on path failure")
Signed-off-by: John Johansen <john.johansen@canonical.com>
get_current_exe_path() takes both an exe_file reference and a path
reference before resolving the path name. If aa_path_name() failed, it
returned immediately and leaked both references.
Route the failure through the common cleanup path so fput() and path_put()
always run after the references are acquired.
Fixes: 8d34e16f7f ("apparmor: userns: Add support for execpath in userns")
Reviewed-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: John Johansen <john.johansen@canonical.com>
unpack_pdb() may need to allocate a missing ACCEPT2 table for older policy
data. If that allocation failed, it set an error message but jumped to the
success path, returning a policydb with the required table missing.
Return -ENOMEM through the normal failure path when the ACCEPT2 allocation
fails. Remove the now-unused out label.
Fixes: 2e12c5f060 ("apparmor: add additional flags to extended permission.")
Reviewed-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Return NULL instead of passing to ERR_PTR while error is zero.
Fixes smatch warning:
- security/apparmor/apparmorfs.c:1846 ns_mkdir_op() warn:
passing zero to 'ERR_PTR'
Fixes: 88d5baf690 ("Change inode_operations.mkdir to return struct dentry *")
Reviewed-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Signed-off-by: John Johansen <john.johansen@canonical.com>
When the export_binary parameter is set, then rawdata is available and
there should be a symbolic link for the rawdata in the profile
directory in apparmorfs. If the parameter is unset, then the symlinks
should not exist.
The issue arises when changing the value of export_binary on runtime
and replacing profiles. If export_binary was set when the profile was
originally loaded, then changed to 0 and the profile was reloaded,
then the symbolic links would still exist but would return ENOENT
because the rawdata no longer exists.
On the opposite side, if export_binary was unset when the profile was
originally loaded, then changed to 1 and the profile was reloaded,
then the symbolic links would not exist, even though the rawdata does.
Fixes: d61c57fde8 ("apparmor: make export of raw binary profile to userspace optional")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
pdb->dfa could be NULL if unpack_dfa fails, causing a NULL pointer
dereference.
Fixes: 2e12c5f060 ("apparmor: add additional flags to extended permission.")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Manuel Diewald <manuel.diewald@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently fn_label_build() callback fns must provide a transition or
failure. Change this so that a callback can indicate it should be
skipped/not be involved in the label being built.
This will be useful when building object labels based on mediation
flags, as to whether the label should be set.
Existing callers can keep treating NULL return as an error because
none of those callback fns support skipping, but instead of the old
error handling replace with AA_BUG.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Previously fn_label_build() was accepting a NULL which represented
ENOMEM return and ERR_PTR for errors.
Clean this up by requiring the cb fn to return an ERR_PTR or valid
value.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The function aa_replace_profiles was accessing udata->size after calling
aa_put_loaddata(udata), causing a potential UAF.
Fixed this by saving the size to a local variable before dropping the
reference.
Fixes: 5ac8c355ae ("apparmor: allow introspecting the loaded policy pre internal transform")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
profiles can be pinned by file and other references, and can live long
after they have been replaced/removed. The rawdata however is no longer
needed, and can be freed earlier than the rest of the profile.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
There was a race condition involving change_hat and profile replacement in
which replacement of the parent profile during a changehat operation could
result in the list of children becoming empty and the changehat operation
failing. To prevent this:
- grab the namespace lock until we've built the hat transition, and
- use aa_get_newest_profile to avoid using stale profile objects.
Link: https://bugs.launchpad.net/bugs/2139664
Fixes: 89dbf1962a ("apparmor: move change_hat mediation to using labels")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
rawdata_f_data has a blob of data that is allocated at its end but
not explicitly declared. Makes sure it is correctly declared as a
flex_rray.
Fixes: 63c16c3a76 ("apparmor: Initial implementation of raw policy blob compression")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The macro is equivalent to OR-ing in the bitflag manually, but using the
macro consistently makes grepping for these occurrences easier.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently, if the `kvzalloc` in `unpack_table` fails, it returns NULL.
This is masked by `aa_dfa_unpack` which interprets NULL as a -EPROTO,
leading to confusing error messages in `apparmor_parser` [1].
The fixed behavior correctly propagates -ENOMEM on allocation failure.
Link: https://gitlab.com/apparmor/apparmor/-/issues/592
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Differential encoding while present has not been made broadly
available, pending further review and testing. Now that has happened
advertise its availability to user space.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa_label_alloc() allocates a secid before allocating or taking the label
proxy. If the later proxy step fails, the error path only freed the label
memory, leaking any resources initialized by aa_label_init().
Use aa_label_free() on the failure path so partially initialized labels
release their secid and other label resources before the backing memory is
freed.
Fixes: f1bd904175 ("apparmor: add the base fns() for domain labels")
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa_change_profile() builds a replacement label with
fn_label_build_in_scope() before the no_new_privs subset check. The build
helper can fail and return NULL or an ERR_PTR, but the result was passed
to aa_label_is_unconfined_subset() before the existing IS_ERR_OR_NULL()
check.
Reuse the existing target-label build failure handling immediately after
the build. This preserves the current audit handling while preventing the
subset helper from dereferencing an invalid label.
Fixes: e00b02bb6a ("apparmor: move change_profile mediation to using labels")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some config did this:
security/apparmor/apparmorfs.c:177:28: warning: 'get_loaddata_common_ref' defined but not used [-Wunused-function]
177 | static struct aa_loaddata *get_loaddata_common_ref(struct aa_common_ref *ref)
get_loaddata_common_ref() is only used if
CONFIG_SECURITY_APPARMOR_EXPORT_BINARY=y.
(Or of course move the function into that block if maintainers perfer)
Fixes: 8e135b8aee ("apparmor: fix race between freeing data and fs accessing it")
Cc: John Johansen <john.johansen@canonical.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
get_newest_label() will always return a refcount, on the profile it
returns. However there are cases where we only need the refcount
if the label is stale and get_newest_label() will return a different
label.
Optimize this by making the get/put happen conditionally, by keeping
a flag indicating if the get was performed and a put is needed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently update_sk_ctx() transfers the plabel reference, unfortunately
it is also unconditionally put in the caller. Ideally we would make
the caller conditionally put the reference based on whether it was
transferred but for now just fix the bug by getting a reference.
Fixes: 88fec3526e ("apparmor: make sure unix socket labeling is correctly updated.")
Signed-off-by: John Johansen <john.johansen@canonical.com>
The holding a reference to the peer_sk is not enough to ensure access
to the peer sk path. Accessing the path outside of the state lock
allows for a race with unix_release_sock(). Fix this by taking the
state lock and getting a reference to the path under lock.
Ideally for connected sockets we would cache this information so we
don't have to take the lock here. But for now just fix the race.
Fixes: bc6e5f6933 ("apparmor: Remove use of the double lock")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Unfortunately the plabel was being shadowed by an unused local var.
This didn't affect the mediation check but did cauase the cache to
not correctly be updated resulting in extra mediation checks.
Fixes: 88fec3526e ("apparmor: make sure unix socket labeling is correctly updated.")
Signed-off-by: John Johansen <john.johansen@canonical.com>
- 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()
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEE7cSDD705q2rFEEf7BS82cBjVw9gFAmnqqQkACgkQBS82cBjV
w9hbkxAAhNJACQhDVcCFHIRzFTRYANEUtqwn2Ry4wyVuJA5N17McAe4JnPfvSuDv
h/jSov5dPNw2gIgnwpX7aOf2ARe3l4FOMx43mUITnmahtwRoKVooV79RwTXHUxBX
NXamG5kEVtz4p4iFR5j2R5LiddwueqtzXbld85ieId3BuoMo8cwklKlDjrMpMNai
mQSwkeTB6Qv/M2r0MiDml0V8OYE4AO00CeEU3/vBZayu9j8xuqY8DvKwcwtbnRmm
7TnITp0EMmtjpNkZHkjXwdAfMKBogYuLSlKwEq33H07cEMAxBbC6gevAVqIBLz3G
e9/CbiQn2l1qZzXoBgrZV8MMJRN9aaKl+wrZmVu4zijlhTOP0AoqZ9q354mCIYA5
k2jLEuZvgr+bggFMHPjfJt6Wa5N9EhoBRikziW6lYmtORqe4ng5Wj6VtVxba82qR
yIGAViresioyHRvgOzbLjavi9q5nkQsPZmPvvMPyD1e2bXTzuw7uktGWCpI32Vsy
Mu0H7CULCpzFAXFyK/Q0fMQaYRchq2vBJ83T5IXfe8vslSH7uKsPuiU6C8/oIA8J
0DPAbVQ/KJ2ArPOGNCUO84xj2Hjw8yANdetjEIC7zHIerRxagSIAbnXBIuuR2EQD
Z5uEgXTchX2PRW+dO+prYYhKufHGy7MTi+geUwHaxb+qefU8dlY=
=OOib
-----END PGP SIGNATURE-----
Merge tag 'apparmor-pr-2026-04-23' of 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()
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: 98b824ff89 ("apparmor: refcount the pdb")
Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
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: 93d4dbdc8d ("apparmor: Replace deprecated strcpy in d_namespace_path")
Signed-off-by: John Johansen <john.johansen@canonical.com>
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: 5e26a01e56 ("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>
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: 3d28e2397a ("apparmor: add support loading per permission tagging")
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
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: c27c6bd2c4 ("apparmor: ensure that dfa state tables have entries")
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
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>
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>
Please consider pulling these changes from the signed vfs-7.1-rc1.kino tag.
Thanks!
Christian
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCadjZCgAKCRCRxhvAZXjc
otmnAP4sbsxZQdz2TG2hJuOwnEZOkkxZQOUMc3ERVyZaWXIeTAEA7e5M+8FpoG9n
8ipO76UoaXdGLESrqVdp9EOhLqOW7QY=
=uMeJ
-----END PGP SIGNATURE-----
Merge tag 'vfs-7.1-rc1.kino' of 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
Please consider pulling these changes from the signed vfs-7.1-rc1.directory tag.
Thanks!
Christian
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCadjZCgAKCRCRxhvAZXjc
oj2yAQDHLWMfxDr8INpBuGc09tqX04qyb7td3U+zfM5c3bvLLAD/eDFSSUvzLtPD
u540EWvTaJBV5ALUx3vQK96PPvS6Vg4=
=20jQ
-----END PGP SIGNATURE-----
Merge tag 'vfs-7.1-rc1.directory' of 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
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: c961ee5f21 ("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>
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: 5d5182cae4 ("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>
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: 031dcc8f4e ("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>
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: b7fd2c0340 ("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>
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: 145a0ef21c ("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>
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: c88d4c7b04 ("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>
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: c88d4c7b04 ("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>
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: dd51c84857 ("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>
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: ad5ff3db53 ("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>
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>
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>
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: dd51c84857 ("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>
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: 3b529a7600 ("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>
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>
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>
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>
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: 3d28e2397a ("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>
if debugging is enabled the DEBUG statement will fail do to a bad
fat fingered cast.
Fixes: 102ada7ca3 ("apparmor: fix fmt string type error in process_strs_entry")
Signed-off-by: John Johansen <john.johansen@canonical.com>
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: a4c9efa4db ("apparmor: make label_match return a consistent value")
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
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>
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>
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: ea9bae12d0 ("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>
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>
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: f1bd904175 ("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>
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>
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>
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: baa73d9e47 ("posix-timers: Make them configurable")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
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>
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/2120439https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1952084https://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: 192ca6b55a ("apparmor: revalidate files during exec")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
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: fd1b2b95a2 ("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>
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>
with the previous changes to mmap the in_atomic flag is now always
false, so drop it.
Suggested-by: Tyler Hicks <code@tyhicks.com>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The previous value of GFP_ATOMIC is an int and not a bool, potentially
resulting in UB when being assigned to a bool. In addition, the mmap hook
is called outside of locks (i.e. in a non-atomic context), so we can pass
a fixed constant value of false instead to common_mmap.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This new field allows reliable identification of the binary that
triggered a denial since the existing field (comm) only gives the name of
the binary, not its path. Thus comm doesn't work for binaries outside of
$PATH or works unreliably when two binaries have the same name.
Additionally comm can be modified by a program, for example, comm="(tor)"
or comm=4143504920506F6C6C6572 (= ACPI Poller).
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for the per permission tag index for a given permission
set. This will be used by both meta-data tagging, to allow annotating
accept states with context and debug information. As well as by rule
tainting and triggers to specify the taint or trigger to be applied.
Since these are low frequency ancillary data items they are stored
in a tighter packed format to that allows for sharing and reuse of the
strings between permissions and accept states. Reducing the amount of
kernel memory use at the cost of having to go through a couple if
index based indirections.
The tags are just strings that has no meaning with out context. When
used as meta-data for auditing and debugging its entirely information
for userspace, but triggers, and tainting can be used to affect the
domain. However they all exist in the same packed data set and can
be shared between different uses.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The strtable is currently limited to a single entry string on unpack
even though domain has the concept of multiple entries within it. Make
this a reality as it will be used for tags and more advanced domain
transitions.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Source blob may come from userspace and might be unaligned.
Try to optize the copying process by avoiding unaligned memory accesses.
- Added Fixes tag
- Added "Fix &" to description as this doesn't just optimize but fixes
a potential unaligned memory access
Fixes: e6e8bf4188 ("apparmor: fix restricted endian type warnings for dfa unpack")
Signed-off-by: Helge Deller <deller@gmx.de>
[jj: remove duplicate word "convert" in comment trigger checkpatch warning]
Signed-off-by: John Johansen <john.johansen@canonical.com>
strcpy() is deprecated; use memcpy() instead. Unlike strcpy(), memcpy()
does not copy the NUL terminator from the source string, which would be
overwritten anyway on every iteration when using strcpy(). snprintf()
then ensures that 'char *s' is NUL-terminated.
Replace the hard-coded path length to remove the magic number 6, and add
a comment explaining the extra 11 bytes.
Closes: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Deal with the potential that sock and sock-sk can be NULL during
socket setup or teardown. This could lead to an oops. The fix for NULL
pointer dereference in __unix_needs_revalidation shows this is at
least possible for af_unix sockets. While the fix for af_unix sockets
applies for newer mediation this is still the fall back path for older
af_unix mediation and other sockets, so ensure it is covered.
Fixes: 56974a6fcf ("apparmor: add base infastructure for socket mediation")
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
When receiving file descriptors via SCM_RIGHTS, both the socket pointer
and the socket's sk pointer can be NULL during socket setup or teardown,
causing NULL pointer dereferences in __unix_needs_revalidation().
This is a regression in AppArmor 5.0.0 (kernel 6.17+) where the new
__unix_needs_revalidation() function was added without proper NULL checks.
The crash manifests as:
BUG: kernel NULL pointer dereference, address: 0x0000000000000018
RIP: aa_file_perm+0xb7/0x3b0 (or +0xbe/0x3b0, +0xc0/0x3e0)
Call Trace:
apparmor_file_receive+0x42/0x80
security_file_receive+0x2e/0x50
receive_fd+0x1d/0xf0
scm_detach_fds+0xad/0x1c0
The function dereferences sock->sk->sk_family without checking if either
sock or sock->sk is NULL first.
Add NULL checks for both sock and sock->sk before accessing sk_family.
Fixes: 88fec3526e ("apparmor: make sure unix socket labeling is correctly updated.")
Reported-by: Jamin Mc <jaminmc@gmail.com>
Closes: https://bugzilla.proxmox.com/show_bug.cgi?id=7083
Closes: https://gitlab.com/apparmor/apparmor/-/issues/568
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: System Administrator <root@localhost>
Signed-off-by: John Johansen <john.johansen@canonical.com>
strcpy() is deprecated; replace it with a direct '/' assignment. The
buffer is already NUL-terminated, so there is no need to copy an
additional NUL terminator as strcpy() did.
Update the comment and add the local variable 'is_root' for clarity.
Closes: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: John Johansen <john.johansen@canonical.com>
strcpy() is deprecated and sprintf() does not perform bounds checking
either. Although an overflow is unlikely, it's better to proactively
avoid it by using the safer strscpy() and scnprintf(), respectively.
Additionally, unify memory allocation for 'hname' to simplify and
improve aa_policy_init().
Closes: https://github.com/KSPP/linux/issues/88
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>
Replace unbounded sprintf() calls with snprintf() to prevent potential
buffer overflows in aa_new_learning_profile(). While the current code
works correctly, snprintf() is safer and follows secure coding best
practices. No functional changes.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: John Johansen <john.johansen@canonical.com>
dentries of created objects in dcache (and undo it when removing those).
Reference is grabbed and not released, but it's not actually _stored_
anywhere. That works, but it's hard to follow and verify; among other
things, we have no way to tell _which_ of the increments is intended
to be an unpaired one. Worse, on removal we need to decide whether
the reference had already been dropped, which can be non-trivial if
that removal is on umount and we need to figure out if this dentry is
pinned due to e.g. unlink() not done. Usually that is handled by using
kill_litter_super() as ->kill_sb(), but there are open-coded special
cases of the same (consider e.g. /proc/self).
Things get simpler if we introduce a new dentry flag (DCACHE_PERSISTENT)
marking those "leaked" dentries. Having it set claims responsibility
for +1 in refcount.
The end result this series is aiming for:
* get these unbalanced dget() and dput() replaced with new primitives that
would, in addition to adjusting refcount, set and clear persistency flag.
* instead of having kill_litter_super() mess with removing the remaining
"leaked" references (e.g. for all tmpfs files that hadn't been removed
prior to umount), have the regular shrink_dcache_for_umount() strip
DCACHE_PERSISTENT of all dentries, dropping the corresponding
reference if it had been set. After that kill_litter_super() becomes
an equivalent of kill_anon_super().
Doing that in a single step is not feasible - it would affect too many places
in too many filesystems. It has to be split into a series.
This work has really started early in 2024; quite a few preliminary pieces
have already gone into mainline. This chunk is finally getting to the
meat of that stuff - infrastructure and most of the conversions to it.
Some pieces are still sitting in the local branches, but the bulk of
that stuff is here.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQQqUNBr3gm4hGXdBJlZ7Krx/gZQ6wUCaTEq1wAKCRBZ7Krx/gZQ
643uAQC1rRslhw5l7OjxEpIYbGG4M+QaadN4Nf5Sr2SuTRaPJQD/W4oj/u4C2eCw
Dd3q071tqyvm/PXNgN2EEnIaxlFUlwc=
=rKq+
-----END PGP SIGNATURE-----
Merge tag 'pull-persistency' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull persistent dentry infrastructure and conversion from Al Viro:
"Some filesystems use a kinda-sorta controlled dentry refcount leak to
pin dentries of created objects in dcache (and undo it when removing
those). A reference is grabbed and not released, but it's not actually
_stored_ anywhere.
That works, but it's hard to follow and verify; among other things, we
have no way to tell _which_ of the increments is intended to be an
unpaired one. Worse, on removal we need to decide whether the
reference had already been dropped, which can be non-trivial if that
removal is on umount and we need to figure out if this dentry is
pinned due to e.g. unlink() not done. Usually that is handled by using
kill_litter_super() as ->kill_sb(), but there are open-coded special
cases of the same (consider e.g. /proc/self).
Things get simpler if we introduce a new dentry flag
(DCACHE_PERSISTENT) marking those "leaked" dentries. Having it set
claims responsibility for +1 in refcount.
The end result this series is aiming for:
- get these unbalanced dget() and dput() replaced with new primitives
that would, in addition to adjusting refcount, set and clear
persistency flag.
- instead of having kill_litter_super() mess with removing the
remaining "leaked" references (e.g. for all tmpfs files that hadn't
been removed prior to umount), have the regular
shrink_dcache_for_umount() strip DCACHE_PERSISTENT of all dentries,
dropping the corresponding reference if it had been set. After that
kill_litter_super() becomes an equivalent of kill_anon_super().
Doing that in a single step is not feasible - it would affect too many
places in too many filesystems. It has to be split into a series.
This work has really started early in 2024; quite a few preliminary
pieces have already gone into mainline. This chunk is finally getting
to the meat of that stuff - infrastructure and most of the conversions
to it.
Some pieces are still sitting in the local branches, but the bulk of
that stuff is here"
* tag 'pull-persistency' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (54 commits)
d_make_discardable(): warn if given a non-persistent dentry
kill securityfs_recursive_remove()
convert securityfs
get rid of kill_litter_super()
convert rust_binderfs
convert nfsctl
convert rpc_pipefs
convert hypfs
hypfs: swich hypfs_create_u64() to returning int
hypfs: switch hypfs_create_str() to returning int
hypfs: don't pin dentries twice
convert gadgetfs
gadgetfs: switch to simple_remove_by_name()
convert functionfs
functionfs: switch to simple_remove_by_name()
functionfs: fix the open/removal races
functionfs: need to cancel ->reset_work in ->kill_sb()
functionfs: don't bother with ffs->ref in ffs_data_{opened,closed}()
functionfs: don't abuse ffs_data_closed() on fs shutdown
convert selinuxfs
...
-----BEGIN PGP SIGNATURE-----
iQJIBAABCgAyFiEES0KozwfymdVUl37v6iDy2pc3iXMFAmkuALUUHHBhdWxAcGF1
bC1tb29yZS5jb20ACgkQ6iDy2pc3iXOtDg/8DMxvN2XKZrryP31zdknUEHLJOTfz
eFCaNKQJK9GpJ1Q/Z4P/q/dH4QUKZHEM7E18N/hjA4Nx6Z7I1eVPK6hvvySkRa9l
b5j+GTLteMcANV04i04B8VTn2mtEW5SZp0Y280EFOMoVGvav72zAt4HHWVytDzyy
tVzvuC6iPNbe7rw+eUzTjHAq3WWWYe42QmiDfnAttdjWloSnfMx6AIvEoeo6jryc
aLGeZQsrgk2wL/ovXXD5kvDo1EQnETGuxQRh8P3W2DzLwEtt6d+BpfAm9PE0FE4k
oE5YrqOhvIpmcBm/8DdkvZ0o0gdfe0IrACvoEqJVpWs6w6T6zusiTzwWp7tBzET/
ygqYabUpz+BrAsGNVtXlDD4va37e5OI500PjDntuT4GMwKBGe5JKXLeki0sQeu6d
AcZd8hu6sVpYDLWJoWDXplxq1ndJTfafVtONQ5Cw8BHM5j6CIAaZM13KG9rJSOYa
uyNOfHxndsjV7dzuQ9S763l4djixiw0oU/PF+XQP4dC/Dyf60yb47mCOlZndRaJj
/FqR0Rbp2KonOSrkmzPTteGJOLMgM5bquZsSHNClxC/qeHTv8xKWf0HRWN61ZUe2
/NLcSjL+CIcN6q0c8jx/k7I9N/yQcmQLQIVTnUY6YOi0TkhUUdqSaq0rp8rSDW9z
AUvHpfPpC92klcM=
=u7yQ
-----END PGP SIGNATURE-----
Merge tag 'lsm-pr-20251201' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm
Pull LSM updates from Paul Moore:
- Rework the LSM initialization code
What started as a "quick" patch to enable a notification event once
all of the individual LSMs were initialized, snowballed a bit into a
30+ patch patchset when everything was done. Most of the patches, and
diffstat, is due to splitting out the initialization code into
security/lsm_init.c and cleaning up some of the mess that was there.
While not strictly necessary, it does cleanup the code signficantly,
and hopefully makes the upkeep a bit easier in the future.
Aside from the new LSM_STARTED_ALL notification, these changes also
ensure that individual LSM initcalls are only called when the LSM is
enabled at boot time. There should be a minor reduction in boot times
for those who build multiple LSMs into their kernels, but only enable
a subset at boot.
It is worth mentioning that nothing at present makes use of the
LSM_STARTED_ALL notification, but there is work in progress which is
dependent upon LSM_STARTED_ALL.
- Make better use of the seq_put*() helpers in device_cgroup
* tag 'lsm-pr-20251201' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm: (36 commits)
lsm: use unrcu_pointer() for current->cred in security_init()
device_cgroup: Refactor devcgroup_seq_show to use seq_put* helpers
lsm: add a LSM_STARTED_ALL notification event
lsm: consolidate all of the LSM framework initcalls
selinux: move initcalls to the LSM framework
ima,evm: move initcalls to the LSM framework
lockdown: move initcalls to the LSM framework
apparmor: move initcalls to the LSM framework
safesetid: move initcalls to the LSM framework
tomoyo: move initcalls to the LSM framework
smack: move initcalls to the LSM framework
ipe: move initcalls to the LSM framework
loadpin: move initcalls to the LSM framework
lsm: introduce an initcall mechanism into the LSM framework
lsm: group lsm_order_parse() with the other lsm_order_*() functions
lsm: output available LSMs when debugging
lsm: cleanup the debug and console output in lsm_init.c
lsm: add/tweak function header comment blocks in lsm_init.c
lsm: fold lsm_init_ordered() into security_init()
lsm: cleanup initialize_lsm() and rename to lsm_init_single()
...
At this point there are very few call chains that might lead to
d_make_discardable() on a dentry that hadn't been made persistent:
calls of simple_unlink() and simple_rmdir() in configfs and
apparmorfs.
Both filesystems do pin (part of) their contents in dcache, but
they are currently playing very unusual games with that. Converting
them to more usual patterns might be possible, but it's definitely
going to be a long series of changes in both cases.
For now the easiest solution is to have both stop using simple_unlink()
and simple_rmdir() - that allows to make d_make_discardable() warn
when given a non-persistent dentry.
Rather than giving them full-blown private copies (with calls of
d_make_discardable() replaced with dput()), let's pull the parts of
simple_unlink() and simple_rmdir() that deal with timestamps and link
counts into separate helpers (__simple_unlink() and __simple_rmdir()
resp.) and have those used by configfs and apparmorfs.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
start_removing_dentry() is similar to start_removing() but instead of
providing a name for lookup, the target dentry is given.
start_removing_dentry() checks that the dentry is still hashed and in
the parent, and if so it locks and increases the refcount so that
end_removing() can be used to finish the operation.
This is used in cachefiles, overlayfs, smb/server, and apparmor.
There will be other users including ecryptfs.
As start_removing_dentry() takes an extra reference to the dentry (to be
put by end_removing()), there is no need to explicitly take an extra
reference to stop d_delete() from using dentry_unlink_inode() to negate
the dentry - as in cachefiles_delete_object(), and ksmbd_vfs_unlink().
cachefiles_bury_object() now gets an extra ref to the victim, which is
drops. As it includes the needed end_removing() calls, the caller
doesn't need them.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20251113002050.676694-9-neilb@ownmail.net
Signed-off-by: Christian Brauner <brauner@kernel.org>
Reduce the duplication between the lsm_id struct and the DEFINE_LSM()
definition by linking the lsm_id struct directly into the individual
LSM's DEFINE_LSM() instance.
Linking the lsm_id into the LSM definition also allows us to simplify
the security_add_hooks() function by removing the code which populates
the lsm_idlist[] array and moving it into the normal LSM startup code
where the LSM list is parsed and the individual LSMs are enabled,
making for a cleaner implementation with less overhead at boot.
Reviewed-by: Kees Cook <kees@kernel.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQQqUNBr3gm4hGXdBJlZ7Krx/gZQ6wUCaN3daAAKCRBZ7Krx/gZQ
6zNWAP9kD6rOJRNqDgea4pibDPa47Tps/WM5tsDv3dsLliY29gEA6sveOWZ3guAj
4oY3ts/NtHLWXvhI7Vd/1mr2aTKEZQk=
=YNK+
-----END PGP SIGNATURE-----
Merge tag 'pull-f_path' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull file->f_path constification from Al Viro:
"Only one thing was modifying ->f_path of an opened file - acct(2).
Massaging that away and constifying a bunch of struct path * arguments
in functions that might be given &file->f_path ends up with the
situation where we can turn ->f_path into an anon union of const
struct path f_path and struct path __f_path, the latter modified only
in a few places in fs/{file_table,open,namei}.c, all for struct file
instances that are yet to be opened"
* tag 'pull-f_path' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (23 commits)
Have cc(1) catch attempts to modify ->f_path
kernel/acct.c: saner struct file treatment
configfs:get_target() - release path as soon as we grab configfs_item reference
apparmor/af_unix: constify struct path * arguments
ovl_is_real_file: constify realpath argument
ovl_sync_file(): constify path argument
ovl_lower_dir(): constify path argument
ovl_get_verity_digest(): constify path argument
ovl_validate_verity(): constify {meta,data}path arguments
ovl_ensure_verity_loaded(): constify datapath argument
ksmbd_vfs_set_init_posix_acl(): constify path argument
ksmbd_vfs_inherit_posix_acl(): constify path argument
ksmbd_vfs_kern_path_unlock(): constify path argument
ksmbd_vfs_path_lookup_locked(): root_share_path can be const struct path *
check_export(): constify path argument
export_operations->open(): constify path argument
rqst_exp_get_by_name(): constify path argument
nfs: constify path argument of __vfs_getattr()
bpf...d_path(): constify path argument
done_path_create(): constify path argument
...
-----BEGIN PGP SIGNATURE-----
iQJIBAABCgAyFiEES0KozwfymdVUl37v6iDy2pc3iXMFAmjWq5oUHHBhdWxAcGF1
bC1tb29yZS5jb20ACgkQ6iDy2pc3iXPTjRAAwYapnw+ZGdFtTGIDT63HtlKjCGHg
DRR8J1RYWhxQL78dInjl7hlGPd4ULdpdF6zsh27X/8OsdFotw4NhDyPJwS1qWZv9
uBJMy/s1Qi1V/rrtDygLGgkQ9ICfl/hgVh3L+g9AXU8H9IapMULp33z+2ueFU4rA
PXgXppgNQTOhIQml0tagY7iPlLaaI1uPv/Dbvt792CSrKZReC+uiDSQKD6SUy5oJ
NBRs0emdCqbllo8Eo7wTGdfzUttsPWYHe7X9BGCMK2bHp0BpMnFBDtuipUAgjNE8
O16EkAtBMpEBW9VEFvDYW1jMFO7ccD8b09CbqPLdE7E0GeigTiODg+FdncKEpZn0
Dl4xPbIoPBHVrDHKFK3HcuEdUs0FZH3NpTLFRg0/nWbg3CfSOFq1ZKhSbwLTZ48V
2Iq22G0hIIl3yTEePSoR8xCSQkWf6hA1SVvzBqw5Xn1tnkdIUuM+KzeZUPKxCOiH
r5b3ufrN5YMAcmc59q393sNuSMd7s97fohhK8/HouB93EcVNM2UjLEKVJnhMhYRE
N21O17jwQG9F+OYTnmtMzuUF6yxwSAmkzQOg6F+lalJ8MECnNrZOEeyuA3d5ISi5
4ZrXHWw90qaDy9lCV1o0UwWt9na+WxeMCJNpI07h5V1k3x7BULiI6WeP7J1qnY9r
YlLv/6Hgx29dtqE=
=iQal
-----END PGP SIGNATURE-----
Merge tag 'audit-pr-20250926' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit
Pull audit updates from Paul Moore:
- Proper audit support for multiple LSMs
As the audit subsystem predated the work to enable multiple LSMs,
some additional work was needed to support logging the different LSM
labels for the subjects/tasks and objects on the system. Casey's
patches add new auxillary records for subjects and objects that
convey the additional labels.
- Ensure fanotify audit events are always generated
Generally speaking security relevant subsystems always generate audit
events, unless explicitly ignored. However, up to this point fanotify
events had been ignored by default, but starting with this pull
request fanotify follows convention and generates audit events by
default.
- Replace an instance of strcpy() with strscpy()
- Minor indentation, style, and comment fixes
* tag 'audit-pr-20250926' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit:
audit: fix skb leak when audit rate limit is exceeded
audit: init ab->skb_list earlier in audit_buffer_alloc()
audit: add record for multiple object contexts
audit: add record for multiple task security contexts
lsm: security_lsmblob_to_secctx module selection
audit: create audit_stamp structure
audit: add a missing tab
audit: record fanotify event regardless of presence of rules
audit: fix typo in auditfilter.c comment
audit: Replace deprecated strcpy() with strscpy()
audit: fix indentation in audit_log_exit()
unix_sk(sock)->path should never be modified, least of all by LSM...
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
With the introduction of clone3 in commit 7f192e3cd3 ("fork: add
clone3") the effective bit width of clone_flags on all architectures was
increased from 32-bit to 64-bit, with a new type of u64 for the flags.
However, for most consumers of clone_flags the interface was not
changed from the previous type of unsigned long.
While this works fine as long as none of the new 64-bit flag bits
(CLONE_CLEAR_SIGHAND and CLONE_INTO_CGROUP) are evaluated, this is still
undesirable in terms of the principle of least surprise.
Thus, this commit fixes all relevant interfaces of callees to
sys_clone3/copy_process (excluding the architecture-specific
copy_thread) to consistently pass clone_flags as u64, so that
no truncation to 32-bit integers occurs on 32-bit architectures.
Signed-off-by: Simon Schuster <schuster.simon@siemens-energy.com>
Link: https://lore.kernel.org/20250901-nios2-implement-clone3-v2-2-53fcf5577d57@siemens-energy.com
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Replace the single skb pointer in an audit_buffer with a list of
skb pointers. Add the audit_stamp information to the audit_buffer as
there's no guarantee that there will be an audit_context containing
the stamp associated with the event. At audit_log_end() time create
auxiliary records as have been added to the list. Functions are
created to manage the skb list in the audit_buffer.
Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
An example of the MAC_TASK_CONTEXTS record is:
type=MAC_TASK_CONTEXTS
msg=audit(1600880931.832:113)
subj_apparmor=unconfined
subj_smack=_
When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record the
"subj=" field in other records in the event will be "subj=?".
An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
multiple security modules that may make access decisions based on a
subject security context.
Refactor audit_log_task_context(), creating a new audit_log_subj_ctx().
This is used in netlabel auditing to provide multiple subject security
contexts as necessary.
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: subj tweak, audit example readability indents]
Signed-off-by: Paul Moore <paul@paul-moore.com>
- 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
- apparmor: 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
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEE7cSDD705q2rFEEf7BS82cBjVw9gFAmiQmakACgkQBS82cBjV
w9jvKBAAn/HblSPo112ycW/yjeonkyiCY56LvyeU1YWQ8m370xPqM3yK2SHcj2i1
we1mx5beDxbH5xn7c6w0EtyoHP7FNhyHp7neG8/WaJ1JG4uxv9HvrDmEJeQEJn/3
5AP1q2dZF9NwnKBfB5zjwZXBJzncWtYBoLUjYMbehWlQjufT2yElyM8YZZN8ziLE
M5ILVX6UMGpBH/zuX5kN2idLcubnv5MvLo2IEt+/nGLPbed44w+mZTM5WOTbzPNq
w8axyNdhGt9kcSGwWuM+48T4oLfwagoxIZ3RXSQ4eExk6I8ZaFXua8nknC9wENN4
9vkzDSWAupQ+o1bLKVNMVkqvBIIqmvEWvwket/hiyxs3Y5PDckRqOgQ/4Wbmgp9B
KhLXxzIrF9PXkZ/rpMzloxnvDtMwoSScDShhW4TCRCmpDo/GwPwoPIpgbnc3kTq0
poomca9KZ7YEnX/90Bh92Duo5OBDOHYlbWVE7EWX01htcxExQJt47JK48C25cY5p
/cVDVepoz7EnKjB7mm9k6K1gYGvDeu3W1whRZNEK74AQJ7p+CrBoU+WjeMmZqP5V
s47cLF17hbnw4ZvfsxQDkPgSOP1kuJIVlwFV2lPQk5hDcT6V0kZtqUzczKJSqeJb
CGOkKvM7ao/7Cn8pmDNG1ZuPl/HuJ6wjlxt7SVt4/3rzLFzwglo=
=Djjn
-----END PGP SIGNATURE-----
Merge tag 'apparmor-pr-2025-08-04' of 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
...
