linux-kernel - Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 23 Aug 2019 17:17:42 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        LKML <linux-kernel@...r.kernel.org>,
        syzbot <syzbot+8ab2d0f39fb79fe6ca40@...kaller.appspotmail.com>,
        Eric Biggers <ebiggers@...nel.org>
Subject: Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.

On 2019/08/23 8:59, Dmitry Vyukov wrote:
>> Can't we introduce a kernel config which selectively blocks specific actions?
>> If we don't need to worry about bypassing blacklist checks, we will be able to
>> enable syz_execute_func() again.
> 
> 
> We can consider this, but we need some set of good use cases first.
> For /dev/{mem,kmem} we disable them with config, right?

/dev/{mem,kmem} can be disabled by kernel config options. But

>                                                         That looks
> like the right thing to do because we don't want fuzzer to do anything
> with these files anyway.

I don't think so. To examine as corner as possible (e.g. lock dependency),
I consider that even doing

----------
+#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
+static char dummybuf[PAGE_SIZE];
+#endif
----------

----------
                        ptr = xlate_dev_mem_ptr(p);
                        if (!ptr) {
                                if (written)
                                        break;
                                return -EFAULT;
                        }
+#ifndef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
                        copied = copy_from_user(ptr, buf, sz);
+#else
+                       copied = copy_from_user(dummybuf, buf, min(sizeof(dummybuf), sz));
+#endif
                        unxlate_dev_mem_ptr(p, ptr);
----------

makes sense, for copy_from_user() might find new lock dependency
which would otherwise be unnoticed.

>                          So this won't be a good use case for
> CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING.
> Fuzzer can also reliably filter out based on syscall numbers of
> top-level argument values. The potential problem is with (1)
> pointers/indirect memory and (2) where blacklisting some top-level
> argument values would backlist too much (e.g. prohibiting 3rd ioctl
> argument 0 entirely).

I consider that functions that freezes processes/filesystems, 
reboots/shutdowns a system, changes console loglevels can be blocked
as well. Trying to examine up to last-second conditional branches will
catch more bugs (e.g. bugs in error recovery paths).