Skip to content

Commit a566a90

Browse files
tych0kees
authored andcommitted
seccomp: don't leak memory when filter install races
In seccomp_set_mode_filter() with TSYNC | NEW_LISTENER, we first initialize the listener fd, then check to see if we can actually use it later in seccomp_may_assign_mode(), which can fail if anyone else in our thread group has installed a filter and caused some divergence. If we can't, we partially clean up the newly allocated file: we put the fd, put the file, but don't actually clean up the *memory* that was allocated at filter->notif. Let's clean that up too. To accomplish this, let's hoist the actual "detach a notifier from a filter" code to its own helper out of seccomp_notify_release(), so that in case anyone adds stuff to init_listener(), they only have to add the cleanup code in one spot. This does a bit of extra locking and such on the failure path when the filter is not attached, but it's a slow failure path anyway. Fixes: 5189149 ("seccomp: allow TSYNC and USER_NOTIF together") Reported-by: syzbot+3ad9614a12f80994c32e@syzkaller.appspotmail.com Signed-off-by: Tycho Andersen <tycho@tycho.pizza> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200902014017.934315-1-tycho@tycho.pizza Signed-off-by: Kees Cook <keescook@chromium.org>
1 parent d012a71 commit a566a90

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

kernel/seccomp.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,13 +1109,12 @@ static long seccomp_set_mode_strict(void)
11091109
}
11101110

11111111
#ifdef CONFIG_SECCOMP_FILTER
1112-
static int seccomp_notify_release(struct inode *inode, struct file *file)
1112+
static void seccomp_notify_detach(struct seccomp_filter *filter)
11131113
{
1114-
struct seccomp_filter *filter = file->private_data;
11151114
struct seccomp_knotif *knotif;
11161115

11171116
if (!filter)
1118-
return 0;
1117+
return;
11191118

11201119
mutex_lock(&filter->notify_lock);
11211120

@@ -1142,6 +1141,13 @@ static int seccomp_notify_release(struct inode *inode, struct file *file)
11421141
kfree(filter->notif);
11431142
filter->notif = NULL;
11441143
mutex_unlock(&filter->notify_lock);
1144+
}
1145+
1146+
static int seccomp_notify_release(struct inode *inode, struct file *file)
1147+
{
1148+
struct seccomp_filter *filter = file->private_data;
1149+
1150+
seccomp_notify_detach(filter);
11451151
__put_seccomp_filter(filter);
11461152
return 0;
11471153
}
@@ -1581,6 +1587,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
15811587
listener_f->private_data = NULL;
15821588
fput(listener_f);
15831589
put_unused_fd(listener);
1590+
seccomp_notify_detach(prepared);
15841591
} else {
15851592
fd_install(listener, listener_f);
15861593
ret = listener;

0 commit comments

Comments
 (0)