[PATCH] fs/open: Fix most outstanding security bugs

[PATCH] fs/open: Fix most outstanding security bugs

[Johannes Thumshirn]

Over the last 20 years, the Linux kernel has accumulated hundreds if not
thousands of security vulnerabilities.

One common pattern in most of these security related reports is processes
called "syzkaller", "trinity" or "syz-executor" opening files and then
abuse kernel interfaces causing kernel crashes or even worse threats using
memory overwrites or by exploiting race conditions.

Hunting down these bugs has become time consuming and very expensive, so
I've decided to put an end to it.

If one of the above mentioned processes tries opening a file, return -EPERM
indicating this process does not have the permission to open files on Linux
anymore.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/open.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/open.c b/fs/open.c
index f1c2f855fd43..3a3b460beccd 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 	struct open_flags op;
 	int fd = build_open_flags(flags, mode, &op);
 	struct filename *tmp;
+	char comm[TASK_COMM_LEN];
+	int i;
+	static const char * const list[] = {
+		"syzkaller",
+		"syz-executor,"
+		"trinity",
+		NULL
+	};
+
+	get_task_comm(comm, current);
+
+	for (i = 0; i < ARRAY_SIZE(list); i++)
+		if (!strncmp(comm, list[i], strlen(list[i])))
+			return -EPERM;
 
 	if (fd)
 		return fd;
-- 
2.16.4

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Matthew Wilcox]

On Mon, Apr 01, 2019 at 11:01:13AM +0200, Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
> 
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
> 
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
> 
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

I think you should have credited Cisco for the idea.

https://twitter.com/RedTeamPT/status/1110843396657238016

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Johannes Thumshirn]

On 01/04/2019 13:22, Matthew Wilcox wrote:
[...]

> 
> I think you should have credited Cisco for the idea.
> 
> https://twitter.com/RedTeamPT/status/1110843396657238016

I actually had the idea longer but they won the race by several days.

Just not sure if their calendar was broken or it was a series attempt.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Mukesh Ojha]

On 4/1/2019 2:31 PM, Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
>
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
>
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
>
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>


Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh

> ---
>   fs/open.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/fs/open.c b/fs/open.c
> index f1c2f855fd43..3a3b460beccd 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
>   	struct open_flags op;
>   	int fd = build_open_flags(flags, mode, &op);
>   	struct filename *tmp;
> +	char comm[TASK_COMM_LEN];
> +	int i;
> +	static const char * const list[] = {
> +		"syzkaller",
> +		"syz-executor,"
> +		"trinity",
> +		NULL
> +	};
> +
> +	get_task_comm(comm, current);
> +
> +	for (i = 0; i < ARRAY_SIZE(list); i++)
> +		if (!strncmp(comm, list[i], strlen(list[i])))
> +			return -EPERM;
>   
>   	if (fd)
>   		return fd;

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Torsten Duwe]

On Mon, Apr 01, 2019 at 11:01:13AM +0200, Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
> 
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
> 
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
> 
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/open.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/open.c b/fs/open.c
> index f1c2f855fd43..3a3b460beccd 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
>  	struct open_flags op;
>  	int fd = build_open_flags(flags, mode, &op);
>  	struct filename *tmp;
> +	char comm[TASK_COMM_LEN];
> +	int i;
> +	static const char * const list[] = {

"list" is a bit ambiguous. You could call it "blacklist" or such.

> +		"syzkaller",
> +		"syz-executor,"
> +		"trinity",
> +		NULL
> +	};
> +
> +	get_task_comm(comm, current);
> +
> +	for (i = 0; i < ARRAY_SIZE(list); i++)
> +		if (!strncmp(comm, list[i], strlen(list[i])))
> +			return -EPERM;
                               ^^^^^^^
should be -ECONNRESET.

Also, I'm missing a sysfs parameter file to add more bad guys dynamically.

>  	if (fd)
>  		return fd;
> -- 
> 2.16.4

But for a start, this is OK.
In any case, as already mentioned, big player Cisco has shown us that this is
definitely the way to go!

Rviewed-by: Torsten Duwe <duwe@lst.de>

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Nikolay Borisov]

On 1.04.19 г. 12:01 ч., Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
> 
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
> 
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
> 
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Ack-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/open.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/open.c b/fs/open.c
> index f1c2f855fd43..3a3b460beccd 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
>  	struct open_flags op;
>  	int fd = build_open_flags(flags, mode, &op);
>  	struct filename *tmp;
> +	char comm[TASK_COMM_LEN];
> +	int i;
> +	static const char * const list[] = {
> +		"syzkaller",
> +		"syz-executor,"
> +		"trinity",
> +		NULL
> +	};
> +
> +	get_task_comm(comm, current);
> +
> +	for (i = 0; i < ARRAY_SIZE(list); i++)
> +		if (!strncmp(comm, list[i], strlen(list[i])))
> +			return -EPERM;
>  
>  	if (fd)
>  		return fd;
> 

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Dmitry Vyukov]

On Mon, Apr 1, 2019 at 4:14 PM Nikolay Borisov <nborisov@suse.com> wrote:
> On 1.04.19 г. 12:01 ч., Johannes Thumshirn wrote:
> > Over the last 20 years, the Linux kernel has accumulated hundreds if not
> > thousands of security vulnerabilities.
> >
> > One common pattern in most of these security related reports is processes
> > called "syzkaller", "trinity" or "syz-executor" opening files and then
> > abuse kernel interfaces causing kernel crashes or even worse threats using
> > memory overwrites or by exploiting race conditions.
> >
> > Hunting down these bugs has become time consuming and very expensive, so
> > I've decided to put an end to it.
> >
> > If one of the above mentioned processes tries opening a file, return -EPERM
> > indicating this process does not have the permission to open files on Linux
> > anymore.
> >
> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
>
> Ack-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Cc: stable@vger.kernel.org # v1.0+

Do we want to extend this to other subsystems?
Should it be a default secomp filter?

> > ---
> >  fs/open.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/fs/open.c b/fs/open.c
> > index f1c2f855fd43..3a3b460beccd 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
> >       struct open_flags op;
> >       int fd = build_open_flags(flags, mode, &op);
> >       struct filename *tmp;
> > +     char comm[TASK_COMM_LEN];
> > +     int i;
> > +     static const char * const list[] = {
> > +             "syzkaller",
> > +             "syz-executor,"
> > +             "trinity",
> > +             NULL
> > +     };
> > +
> > +     get_task_comm(comm, current);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(list); i++)
> > +             if (!strncmp(comm, list[i], strlen(list[i])))
> > +                     return -EPERM;
> >
> >       if (fd)
> >               return fd;
> >

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Eric Biggers]

On Mon, Apr 01, 2019 at 11:01:13AM +0200, Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
> 
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
> 
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
> 
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/open.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/open.c b/fs/open.c
> index f1c2f855fd43..3a3b460beccd 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
>  	struct open_flags op;
>  	int fd = build_open_flags(flags, mode, &op);
>  	struct filename *tmp;
> +	char comm[TASK_COMM_LEN];
> +	int i;
> +	static const char * const list[] = {
> +		"syzkaller",
> +		"syz-executor,"
> +		"trinity",
> +		NULL
> +	};
> +
> +	get_task_comm(comm, current);
> +
> +	for (i = 0; i < ARRAY_SIZE(list); i++)
> +		if (!strncmp(comm, list[i], strlen(list[i])))
> +			return -EPERM;
>  
>  	if (fd)
>  		return fd;
> -- 
> 2.16.4
> 

I like that this can't be circumvented by changing the process name, because it
dereferences a NULL pointer when it gets to the end of the list.  However, I'm a
bit uneasy with updating just this syscall. I suggest we go farther and just do:

diff --git a/init/main.c b/init/main.c
index 598e278b46f7..24f4422cb3c9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -547,6 +547,8 @@ asmlinkage __visible void __init start_kernel(void)
 	char *command_line;
 	char *after_dashes;
 
+	return;
+
 	set_task_stack_end_magic(&init_task);
 	smp_setup_processor_id();
 	debug_objects_early_init();


Please also make sure to add a Reported-by line for each one of the 475 open
syzbot bug reports, so they get properly closed.  Thanks!

- Eric

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Darrick J. Wong]

On Mon, Apr 01, 2019 at 11:01:13AM +0200, Johannes Thumshirn wrote:
> Over the last 20 years, the Linux kernel has accumulated hundreds if not
> thousands of security vulnerabilities.
> 
> One common pattern in most of these security related reports is processes
> called "syzkaller", "trinity" or "syz-executor" opening files and then
> abuse kernel interfaces causing kernel crashes or even worse threats using
> memory overwrites or by exploiting race conditions.
> 
> Hunting down these bugs has become time consuming and very expensive, so
> I've decided to put an end to it.
> 
> If one of the above mentioned processes tries opening a file, return -EPERM
> indicating this process does not have the permission to open files on Linux
> anymore.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/open.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/open.c b/fs/open.c
> index f1c2f855fd43..3a3b460beccd 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
>  	struct open_flags op;
>  	int fd = build_open_flags(flags, mode, &op);
>  	struct filename *tmp;
> +	char comm[TASK_COMM_LEN];
> +	int i;
> +	static const char * const list[] = {
> +		"syzkaller",
> +		"syz-executor,"
> +		"trinity",
> +		NULL
> +	};

This makes no sense, why would you gate on "syz-executor,trinity"?

> +
> +	get_task_comm(comm, current);
> +
> +	for (i = 0; i < ARRAY_SIZE(list); i++)
> +		if (!strncmp(comm, list[i], strlen(list[i])))
> +			return -EPERM;

This is insufficient, because this isn't the only way to open a file.

Wouldn't it be far more effective to use copy_to_user to inject
shellcode into the syzkaller image and change the return address, to
find all the places where syzbot doesn't validate itself sufficiently?
In Soviet Russia, the kernel syzkallz you.

NAK.

--D

>  
>  	if (fd)
>  		return fd;
> -- 
> 2.16.4
> 

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Kees Cook]

On Mon, Apr 1, 2019 at 10:50 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Mon, Apr 1, 2019 at 4:14 PM Nikolay Borisov <nborisov@suse.com> wrote:
> > On 1.04.19 г. 12:01 ч., Johannes Thumshirn wrote:
> > > One common pattern in most of these security related reports is processes
> > > called "syzkaller", "trinity" or "syz-executor" opening files and then
> > > abuse kernel interfaces causing kernel crashes or even worse threats using
> > > memory overwrites or by exploiting race conditions.
> > >
> > > Hunting down these bugs has become time consuming and very expensive, so
> > > I've decided to put an end to it.
> > >
> > > If one of the above mentioned processes tries opening a file, return -EPERM
> > > indicating this process does not have the permission to open files on Linux
> > > anymore.
> > >
> > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> >
> > Ack-by: Nikolay Borisov <nborisov@suse.com>
>
> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: stable@vger.kernel.org # v1.0+
>
> Do we want to extend this to other subsystems?
> Should it be a default secomp filter?

Yes, this protection could make those processes fail too many syscalls. I
agree: it would be better to have all syscalls return 0 with a seccomp
filter. I have updated the patch:


Subject: [PATCH] exec: Fix most outstanding security bugs
From: Kees Cook <keescook@chromium.org>

Over the last 20 years, the Linux kernel has accumulated hundreds if
not thousands of security vulnerabilities.

One common pattern in most of these security related reports is processes
called "syzkaller", "trinity" or "syz-executor" opening files and then
abuse kernel interfaces causing kernel crashes or even worse threats
using memory overwrites or by exploiting race conditions.

Hunting down these bugs has become time consuming and very expensive,
so we've decided to put an end to it.

If one of the above mentioned processes tries to execute a system
call, the call will be skipped but will return zero. This will keep
the process happy without changing kernel state, keeping things safe.

Co-developed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Suggested-by: Dmitry Vyukov <dvyukov@google.com>
Singed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/exec.c               |  1 +
 include/linux/seccomp.h |  2 ++
 kernel/seccomp.c        | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index 2e0033348d8e..c0a73c8e22ff 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1393,6 +1393,7 @@ void finalize_exec(struct linux_binprm *bprm)
 	task_lock(current->group_leader);
 	current->signal->rlim[RLIMIT_STACK] = bprm->rlim_stack;
 	task_unlock(current->group_leader);
+	seccomp_default(current);
 }
 EXPORT_SYMBOL(finalize_exec);
 
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index 84868d37b35d..2d98cd0d79ed 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -45,6 +45,7 @@ extern void secure_computing_strict(int this_syscall);
 
 extern long prctl_get_seccomp(void);
 extern long prctl_set_seccomp(unsigned long, void __user *);
+extern void seccomp_default(struct task_struct *task);
 
 static inline int seccomp_mode(struct seccomp *s)
 {
@@ -73,6 +74,7 @@ static inline long prctl_set_seccomp(unsigned long arg2, char __user *arg3)
 {
 	return -EINVAL;
 }
+static inline void seccomp_default(struct task_struct *task) { }
 
 static inline int seccomp_mode(struct seccomp *s)
 {
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 54a0347ca812..92faee4cded4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1395,6 +1395,45 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
 	return do_seccomp(op, flags, uargs);
 }
 
+/* Certain processes should see all syscalls succeed. */
+void seccomp_default(struct task_struct *task)
+{
+	struct sock_filter filter[] = {
+		BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ERRNO | 0),
+	};
+	struct sock_fprog prog = {
+		.len = ARRAY_SIZE(filter),
+		.filter = filter,
+	};
+	static const char * const list[] = {
+		"syzkaller",
+		"syz-executor",
+		"trinity",
+	};
+	char comm[TASK_COMM_LEN];
+	mm_segment_t old_fs;
+	bool found = false;
+	int i;
+
+	get_task_comm(comm, task);
+
+	for (i = 0; i < ARRAY_SIZE(list); i++) {
+		if (!strncmp(comm, list[i], strlen(list[i]))) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		return;
+
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	seccomp_set_mode_filter(0, (void * __user)&prog);
+	set_fs(old_fs);
+}
+EXPORT_SYMBOL_GPL(seccomp_default);
+
 /**
  * prctl_set_seccomp: configures current->seccomp.mode
  * @seccomp_mode: requested mode to use
-- 
2.17.1


-- 
Kees Cook

Re: [PATCH] fs/open: Fix most outstanding security bugs

[Jessica Yu]

+++ Johannes Thumshirn [01/04/19 11:01 +0200]:
>Over the last 20 years, the Linux kernel has accumulated hundreds if not
>thousands of security vulnerabilities.
>
>One common pattern in most of these security related reports is processes
>called "syzkaller", "trinity" or "syz-executor" opening files and then
>abuse kernel interfaces causing kernel crashes or even worse threats using
>memory overwrites or by exploiting race conditions.
>
>Hunting down these bugs has become time consuming and very expensive, so
>I've decided to put an end to it.
>
>If one of the above mentioned processes tries opening a file, return -EPERM
>indicating this process does not have the permission to open files on Linux
>anymore.
>
>Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Acked-by: Jessica Yu <jeyu@kernel.org>

:-)

>---
> fs/open.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/fs/open.c b/fs/open.c
>index f1c2f855fd43..3a3b460beccd 100644
>--- a/fs/open.c
>+++ b/fs/open.c
>@@ -1056,6 +1056,20 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
> 	struct open_flags op;
> 	int fd = build_open_flags(flags, mode, &op);
> 	struct filename *tmp;
>+	char comm[TASK_COMM_LEN];
>+	int i;
>+	static const char * const list[] = {
>+		"syzkaller",
>+		"syz-executor,"
>+		"trinity",
>+		NULL
>+	};
>+
>+	get_task_comm(comm, current);
>+
>+	for (i = 0; i < ARRAY_SIZE(list); i++)
>+		if (!strncmp(comm, list[i], strlen(list[i])))
>+			return -EPERM;
>
> 	if (fd)
> 		return fd;
>-- 
>2.16.4
>