mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-07-23 07:27:30 -04:00
A deadlock occurs in the audit subsystem when duplicating
executable-related rules.
When a file is moved (e.g., via do_renameat2()), the VFS layer locks
the parent directory (I_MUTEX_PARENT), which synchronously triggers an
fsnotify_move event. If an existing executable audit rule matches the
file being moved, the audit subsystem catches this event and calls
audit_dupe_exe() to duplicate the watch and update the rule. Then,
audit_alloc_mark() would call kern_path_parent() to resolve the path,
leading to a blind attempt to acquire the exact same I_MUTEX_PARENT lock
already held by the task, resulting in the following recursive locking
deadlock:
============================================
WARNING: possible recursive locking detected
6.12.0-55.27.1.el10_0.x86_64+debug #1 Not tainted
--------------------------------------------
mv/5099 is trying to acquire lock:
ffff888132845358 (&inode->i_sb->s_type->i_mutex_dir_key/1){+.+.}-{3:3},
at: __kern_path_locked+0x10a/0x2f0
but task is already holding lock:
ffff888132846b58 (&inode->i_sb->s_type->i_mutex_dir_key/1){+.+.}-{3:3},
at: lock_two_directories+0x13f/0x2b0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&inode->i_sb->s_type->i_mutex_dir_key/1);
lock(&inode->i_sb->s_type->i_mutex_dir_key/1);
*** DEADLOCK ***
May be due to missing lock nesting notation
6 locks held by mv/5099:
#0: ffff888112a9c440 (sb_writers#13)
at: do_renameat2+0x34c/0xbc0
#1: ffff888112a9c790 (&type->s_vfs_rename_key#3)
at: do_renameat2+0x415/0xbc0
#2: ffff888132846b58 (&inode->i_sb->s_type->i_mutex_dir_key/1)
at: lock_two_directories+0x13f/0x2b0
#3: ffff888132845358 (&inode->i_sb->s_type->i_mutex_dir_key/5)
at: lock_two_directories+0x175/0x2b0
#4: ffffffffb3a1fb10 (&fsnotify_mark_srcu)
at: fsnotify+0x454/0x28a0
#5: ffffffffaf886230 (audit_filter_mutex)
at: audit_update_watch+0x36/0x11e0
stack backtrace:
Call Trace:
<TASK>
dump_stack_lvl+0x6f/0xb0
print_deadlock_bug.cold+0xbd/0xca
validate_chain+0x83a/0xf00
__lock_acquire+0xcac/0x1d20
lock_acquire.part.0+0x11b/0x360
down_write_nested+0x9f/0x230
__kern_path_locked+0x10a/0x2f0
kern_path_locked+0x26/0x40
audit_alloc_mark+0xfb/0x4f0
audit_dupe_exe+0x6c/0xe0
audit_dupe_rule+0x6c2/0xc00
audit_update_watch+0x4cc/0x11e0
audit_watch_handle_event+0x12c/0x1b0
send_to_group+0x5d0/0x8b0
fsnotify+0x615/0x28a0
fsnotify_move+0x1d8/0x630
vfs_rename+0xdcd/0x1df0
do_renameat2+0x9d4/0xbc0
__x64_sys_renameat+0x192/0x260
do_syscall_64+0x92/0x180
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7f0491fe8c4e
Code: 0f 1f 40 00 48 8b 15 c1 e1 16 00 f7 d8 64 89 02 b8 ff ff ff ff
c3 66 0f 1f 44 00 00 f3 0f 1e fa 49 89 ca b8 08 01 00 00 0f 05 <48>
3d 00 f0 ff ff 77 0a c3 66 0f 1f 84 00 00 00 00 00 48 8b 15 89
RSP: 002b:00007ffc7210bf38 EFLAGS: 00000246 ORIG_RAX: 0000000000000108
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f0491fe8c4e
RDX: 0000000000000003 RSI: 00007ffc7210e6c8 RDI: 00000000ffffff9c
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
R10: 00005575eb2dae2a R11: 0000000000000246 R12: 00005575eb2dae2a
R13: 00007ffc7210e6c8 R14: 0000000000000003 R15: 00000000ffffff9c
</TASK>
The aforementioned deadlock can be consistently reproduced by running
the script below:
audit-dupe-exe-deadlock.sh
--------------------------
#!/bin/bash
auditctl -D
mkdir -p /tmp/foo
touch /tmp/file
auditctl -a always,exit -F exe=/tmp/file -F path=/tmp/file -S all -k dr
mv /tmp/file /tmp/foo/file
rm -Rf /tmp/foo
This patch fixes the issue by introducing struct audit_watch_ctx to pass
the fsnotify event context down to audit_alloc_mark(). By utilizing the
already-resolved directory inode provided by the event, we bypass the
kern_path_parent() path resolution entirely, safely avoiding the
recursive lock. Furthermore, it explicitly allows duplicate fsnotify
marks (allow_dups = 1) during the rename update, allowing the new rule's
mark to safely coexist with the old rule's mark until the old rule is
freed.
P.S.: This issue was identified and reproduced during a comprehensive
code coverage analysis of the audit subsystem. The full report is
available at the link below:
https://people.redhat.com/rrobaina/audit-code-coverage-analysis.pdf
P.P.S: With the permission of both Ricardo and Nathan, I've squashed a
fixup patch from Nathan that addresses a compile time error when
CONFIG_AUDITSYSCALL=n.
Cc: stable@kernel.org
Fixes: 34d99af52a ("audit: implement audit by executable")
Acked-by: Waiman Long <longman@redhat.com>
Acked-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
[PM: move link metadata into the msg, apply fix from NC]
Signed-off-by: Paul Moore <paul@paul-moore.com>
205 lines
5.4 KiB
C
205 lines
5.4 KiB
C
// SPDX-License-Identifier: GPL-2.0-or-later
|
|
/* audit_fsnotify.c -- tracking inodes
|
|
*
|
|
* Copyright 2003-2009,2014-2015 Red Hat, Inc.
|
|
* Copyright 2005 Hewlett-Packard Development Company, L.P.
|
|
* Copyright 2005 IBM Corporation
|
|
*/
|
|
|
|
#include <linux/kernel.h>
|
|
#include <linux/audit.h>
|
|
#include <linux/kthread.h>
|
|
#include <linux/mutex.h>
|
|
#include <linux/fs.h>
|
|
#include <linux/fsnotify_backend.h>
|
|
#include <linux/namei.h>
|
|
#include <linux/netlink.h>
|
|
#include <linux/sched.h>
|
|
#include <linux/slab.h>
|
|
#include <linux/security.h>
|
|
#include "audit.h"
|
|
|
|
/*
|
|
* this mark lives on the parent directory of the inode in question.
|
|
* but dev, ino, and path are about the child
|
|
*/
|
|
struct audit_fsnotify_mark {
|
|
dev_t dev; /* associated superblock device */
|
|
u64 ino; /* associated inode number */
|
|
char *path; /* insertion path */
|
|
struct fsnotify_mark mark; /* fsnotify mark on the inode */
|
|
struct audit_krule *rule;
|
|
};
|
|
|
|
/* fsnotify handle. */
|
|
static struct fsnotify_group *audit_fsnotify_group;
|
|
|
|
/* fsnotify events we care about. */
|
|
#define AUDIT_FS_EVENTS (FS_MOVE | FS_CREATE | FS_DELETE | FS_DELETE_SELF |\
|
|
FS_MOVE_SELF)
|
|
|
|
static void audit_fsnotify_mark_free(struct audit_fsnotify_mark *audit_mark)
|
|
{
|
|
kfree(audit_mark->path);
|
|
kfree(audit_mark);
|
|
}
|
|
|
|
static void audit_fsnotify_free_mark(struct fsnotify_mark *mark)
|
|
{
|
|
struct audit_fsnotify_mark *audit_mark;
|
|
|
|
audit_mark = container_of(mark, struct audit_fsnotify_mark, mark);
|
|
audit_fsnotify_mark_free(audit_mark);
|
|
}
|
|
|
|
char *audit_mark_path(struct audit_fsnotify_mark *mark)
|
|
{
|
|
return mark->path;
|
|
}
|
|
|
|
int audit_mark_compare(struct audit_fsnotify_mark *mark, u64 ino, dev_t dev)
|
|
{
|
|
if (mark->ino == AUDIT_INO_UNSET)
|
|
return 0;
|
|
return (mark->ino == ino) && (mark->dev == dev);
|
|
}
|
|
|
|
static void audit_update_mark(struct audit_fsnotify_mark *audit_mark,
|
|
const struct inode *inode)
|
|
{
|
|
audit_mark->dev = inode ? inode->i_sb->s_dev : AUDIT_DEV_UNSET;
|
|
audit_mark->ino = inode ? inode->i_ino : AUDIT_INO_UNSET;
|
|
}
|
|
|
|
struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule *krule, char *pathname,
|
|
int len, struct audit_watch_ctx *ctx)
|
|
{
|
|
struct audit_fsnotify_mark *audit_mark;
|
|
struct path path;
|
|
struct dentry *dentry;
|
|
struct inode *dir, *child;
|
|
int ret, allow_dups;
|
|
|
|
if (pathname[0] != '/' || pathname[len-1] == '/')
|
|
return ERR_PTR(-EINVAL);
|
|
|
|
if (!ctx) {
|
|
dentry = kern_path_parent(pathname, &path);
|
|
if (IS_ERR(dentry))
|
|
return ERR_CAST(dentry); /* returning an error */
|
|
dir = d_inode(path.dentry);
|
|
child = d_inode(dentry);
|
|
allow_dups = 0;
|
|
} else {
|
|
dir = ctx->dir;
|
|
child = ctx->child;
|
|
allow_dups = 1;
|
|
}
|
|
|
|
audit_mark = kzalloc_obj(*audit_mark);
|
|
if (unlikely(!audit_mark)) {
|
|
audit_mark = ERR_PTR(-ENOMEM);
|
|
goto out;
|
|
}
|
|
|
|
fsnotify_init_mark(&audit_mark->mark, audit_fsnotify_group);
|
|
audit_mark->mark.mask = AUDIT_FS_EVENTS;
|
|
audit_mark->path = pathname;
|
|
audit_mark->rule = krule;
|
|
|
|
audit_update_mark(audit_mark, child);
|
|
ret = fsnotify_add_inode_mark(&audit_mark->mark, dir, allow_dups);
|
|
|
|
if (ret < 0) {
|
|
audit_mark->path = NULL;
|
|
fsnotify_put_mark(&audit_mark->mark);
|
|
audit_mark = ERR_PTR(ret);
|
|
}
|
|
out:
|
|
if (!ctx) {
|
|
dput(dentry);
|
|
path_put(&path);
|
|
}
|
|
return audit_mark;
|
|
}
|
|
|
|
static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, char *op)
|
|
{
|
|
struct audit_buffer *ab;
|
|
struct audit_krule *rule = audit_mark->rule;
|
|
|
|
if (!audit_enabled)
|
|
return;
|
|
ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
|
|
if (unlikely(!ab))
|
|
return;
|
|
audit_log_session_info(ab);
|
|
audit_log_format(ab, " op=%s path=", op);
|
|
audit_log_untrustedstring(ab, audit_mark->path);
|
|
audit_log_key(ab, rule->filterkey);
|
|
audit_log_format(ab, " list=%d res=1", rule->listnr);
|
|
audit_log_end(ab);
|
|
}
|
|
|
|
void audit_remove_mark(struct audit_fsnotify_mark *audit_mark)
|
|
{
|
|
fsnotify_destroy_mark(&audit_mark->mark, audit_fsnotify_group);
|
|
fsnotify_put_mark(&audit_mark->mark);
|
|
}
|
|
|
|
void audit_remove_mark_rule(struct audit_krule *krule)
|
|
{
|
|
struct audit_fsnotify_mark *mark = krule->exe;
|
|
|
|
audit_remove_mark(mark);
|
|
}
|
|
|
|
static void audit_autoremove_mark_rule(struct audit_fsnotify_mark *audit_mark)
|
|
{
|
|
struct audit_krule *rule = audit_mark->rule;
|
|
struct audit_entry *entry = container_of(rule, struct audit_entry, rule);
|
|
|
|
audit_mark_log_rule_change(audit_mark, "autoremove_rule");
|
|
audit_del_rule(entry);
|
|
}
|
|
|
|
/* Update mark data in audit rules based on fsnotify events. */
|
|
static int audit_mark_handle_event(struct fsnotify_mark *inode_mark, u32 mask,
|
|
struct inode *inode, struct inode *dir,
|
|
const struct qstr *dname, u32 cookie)
|
|
{
|
|
struct audit_fsnotify_mark *audit_mark;
|
|
|
|
audit_mark = container_of(inode_mark, struct audit_fsnotify_mark, mark);
|
|
|
|
if (WARN_ON_ONCE(inode_mark->group != audit_fsnotify_group))
|
|
return 0;
|
|
|
|
if (mask & (FS_CREATE|FS_MOVED_TO|FS_DELETE|FS_MOVED_FROM)) {
|
|
if (audit_compare_dname_path(dname, audit_mark->path, AUDIT_NAME_FULL))
|
|
return 0;
|
|
audit_update_mark(audit_mark, inode);
|
|
} else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF)) {
|
|
audit_autoremove_mark_rule(audit_mark);
|
|
}
|
|
|
|
return 0;
|
|
}
|
|
|
|
static const struct fsnotify_ops audit_mark_fsnotify_ops = {
|
|
.handle_inode_event = audit_mark_handle_event,
|
|
.free_mark = audit_fsnotify_free_mark,
|
|
};
|
|
|
|
static int __init audit_fsnotify_init(void)
|
|
{
|
|
audit_fsnotify_group = fsnotify_alloc_group(&audit_mark_fsnotify_ops,
|
|
FSNOTIFY_GROUP_DUPS);
|
|
if (IS_ERR(audit_fsnotify_group)) {
|
|
audit_fsnotify_group = NULL;
|
|
audit_panic("cannot create audit fsnotify group");
|
|
}
|
|
return 0;
|
|
}
|
|
device_initcall(audit_fsnotify_init);
|