The function calculates new_len as len << 1 for hex encoding. This
has two overflow risks: the shift itself can overflow when len is
large, and the result can be truncated when assigned to new_len
(declared as int) from the size_t calculation.
Fix by using check_shl_overflow() to catch shift overflow and
changing new_len and loop counter i to size_t to prevent truncation.
Cc: stable@vger.kernel.org
Fixes: 168b7173959f ("AUDIT: Clean up logging of untrusted strings")
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
[PM: remove vertical whitspace noise]
Signed-off-by: Paul Moore <paul@paul-moore.com>
Multiple readers access audit_queue.qlen via skb_queue_len() without
holding the queue lock or using READ_ONCE(), while kauditd writes to
this field via the skb_dequeue() → __skb_unlink() path with WRITE_ONCE()
protected by a spinlock. This constitutes data races.
All affected skb_queue_len(&audit_queue) call sites:
- kauditd_thread() wait_event_freezable() condition
- audit_receive_msg() AUDIT_GET handler (s.backlog assignment)
- audit_receive() backlog check
- audit_log_start() backlog check and pr_warn()
KCSAN reports the following conflicting access pattern (one example):
==================================================================
BUG: KCSAN: data-race in audit_log_start / skb_dequeue
write (marked) to 0xffffffff8512ee20 of 4 bytes by task 661 on cpu 57:
skb_dequeue+0x70/0xf0
kauditd_send_queue+0x71/0x220
kauditd_thread+0x1cb/0x430
kthread+0x1c2/0x210
ret_from_fork+0x162/0x1a0
ret_from_fork_asm+0x1a/0x30
read to 0xffffffff8512ee20 of 4 bytes by task 36586 on cpu 1:
audit_log_start+0x2a0/0x6b0
audit_core_dumps+0x64/0xa0
do_coredump+0x14b/0x1260
get_signal+0xeb2/0xf70
arch_do_signal_or_restart+0x41/0x170
exit_to_user_mode_loop+0xa2/0x1c0
do_syscall_64+0x1a3/0x1c0
entry_SYSCALL_64_after_hwframe+0x76/0xe0
value changed: 0x00000001 -> 0x00000000
==================================================================
Resolve the race by switching to lockless helper skb_queue_len_lockless(),
which internally uses READ_ONCE() and properly pairs with the WRITE_ONCE()
write accesses already present on the writer side.
Cc: stable@vger.kernel.org
Fixes: 3197542482df ("audit: rework audit_log_start()")
Signed-off-by: Chi Wang <wangchi@kylinos.cn>
Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
[PM: line length tweak]
Signed-off-by: Paul Moore <paul@paul-moore.com>
Pull audit updates from Paul Moore: - Fix a recursive deadlock when duplicating executable file rules Avoid multiple lookups and attempted I_MUTEX_PARENT locks when moving watched files by passing the already resolved inodes through the audit code. - Fix removal of executable watch rules after the file is deleted Prior to this fix we were unable to remove an executable file watch where the file had been previously deleted due to a negative dentry check in the code that performs the lookup on the file watches. - Convert our basic "unsigned" type usage to "unsigned int". * tag 'audit-pr-20260615' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit: audit: fix recursive locking deadlock in audit_dupe_exe() audit: fix removal of dangling executable rules audit: use 'unsigned int' instead of 'unsigned'
Address checkpatch.pl warning below, across the audit subsystem: WARNING: Prefer 'unsigned int' to bare use of 'unsigned' Minor cleanup, no functional changes. Signed-off-by: Ricardo Robaina <rrobaina@redhat.com> Signed-off-by: Paul Moore <paul@paul-moore.com>
AUDIT_ADD_RULE and AUDIT_DEL_RULE correctly check for AUDIT_LOCKED and return -EPERM, but AUDIT_TRIM and AUDIT_MAKE_EQUIV do not. This allows a process with CAP_AUDIT_CONTROL to modify directory tree watches and equivalence mappings even when the audit configuration has been locked, undermining the purpose of the lock. Add AUDIT_LOCKED checks to both commands. Cc: stable@vger.kernel.org Reviewed-by: Ricardo Robaina <rrobaina@redhat.com> Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Sergio Correia <scorreia@redhat.com> Signed-off-by: Paul Moore <paul@paul-moore.com>
Currently, audit_receive_msg() ignores unknown status bits in AUDIT_SET requests, incorrectly returning success to newer user space tools querying unsupported features. This breaks forward compatibility. Fix this by defining AUDIT_STATUS_ALL and returning -EINVAL if any unrecognized bits are set (s.mask & ~AUDIT_STATUS_ALL). This ensures invalid requests are safely rejected, allowing user space to reliably test for and gracefully handle feature detection on older kernels. Suggested-by: Steve Grubb <sgrubb@redhat.com> Signed-off-by: Ricardo Robaina <rrobaina@redhat.com> [PM: subject line tweak] Signed-off-by: Paul Moore <paul@paul-moore.com>
Static variables are automatically initialized to 0 by the compiler. Remove the redundant explicit assignments in kernel/audit.c to clean up the code, align with standard kernel coding style, and fix the following checkpatch.pl errors: ./scripts/checkpatch.pl kernel/audit.c | grep -A2 ERROR: ERROR: do not initialise statics to 0 + static unsigned long last_check = 0; -- ERROR: do not initialise statics to 0 + static int messages = 0; -- ERROR: do not initialise statics to 0 + static unsigned long last_msg = 0; Signed-off-by: Ricardo Robaina <rrobaina@redhat.com> Signed-off-by: Paul Moore <paul@paul-moore.com>
This is the exact same thing as the 'alloc_obj()' version, only much smaller because there are a lot fewer users of the *alloc_flex() interface. As with alloc_obj() version, this was done entirely with mindless brute force, using the same script, except using 'flex' in the pattern rather than 'objs*'. 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>