linux-kernel - Re: [PATCHv3] kcov: reject open when kernel not instrumented

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, 17 Jun 2016 17:46:38 +0200
From:	Alexander Potapenko <glider@...gle.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	Mark Rutland <mark.rutland@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	James Morse <james.morse@....com>,
	Michal Marek <mmarek@...e.com>
Subject: Re: [PATCHv3] kcov: reject open when kernel not instrumented

On Fri, Jun 17, 2016 at 5:42 PM, Kees Cook <keescook@...omium.org> wrote:
> On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland <mark.rutland@....com> wrote:
>> If the toolchain does not support -fsanitize-coverage=trace-pc, we blat
>> this option from CFLAGS_KCOV, and build the kernel without
>> instrumentation, even if CONFIG_KCOV was selected. However, we still
>> build the rest of the kcov infrastructure, and expose a kcov file under
>> debugfs. This can be confusing, as the kernel will appear to support
>> kcov, yet will never manage to sample any trace PC values. While we do
>> note this fact at build time, this may be missed, and a user may not
>> have access to build logs.
>
> Do you want to refuse to build if the compiler doesn't support the
> flag? I finally figured out how to do this, I think, for
> -fstack-protector:
>
> http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/commit/?h=kbuild/stackprotector&id=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52
>
> If you wanted, the CONFIG_KCOV test could live under the same
> prepare-compiler-check target.
>
> -Kees
>
I suspect the intention here is not to abort the build, but to have an
opportunity to report at run time that the kernel has been built
without kcov support.
>>
>> This patch ensures that CC_HAVE_SANCOV_TRACE_PC is defined when the
>> toolchain supports -fsanitize-coverage=trace-pc, and is not defined
>> otherwise. When CC_HAVE_SANCOV_TRACE_PC is not defined, the kernel will
>> return -ENOTSUPP if userspace attempts to open the kcov debugfs file,
>> indicating that kcov functionality is unavailable.
>>
>> As uninstrumented files (e.g. kernel/kcov.c) need to know when this
>> compiler feature is in use, wee pass the define via KBUILD_CFLAGS rather
>> than CFLAGS_KCOV.
>>
>> Signed-off-by: Mark Rutland <mark.rutland@....com>
>> Cc: Alexander Potapenko <glider@...gle.com>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Cc: Dmitry Vyukov <dvyukov@...gle.com>
>> Cc: James Morse <james.morse@....com>
>> Cc: Kees Cook <keescook@...omium.org>
>> Cc: Michal Marek <mmarek@...e.com>
>> Cc: linux-kernel@...r.kernel.org
>> ---
>>  Makefile      | 2 ++
>>  kernel/kcov.c | 9 +++++++++
>>  2 files changed, 11 insertions(+)
>>
>> Since v1 [1]:
>> * Use CC_HAVE_SANCOV_TRACE_PC rather than CONFIG_KCOV_CC
>> Since v2 [2]:
>> * Use KBUILD_CFLAGS so kernel/kcov.c gets the flag
>>
>> [1] http://lkml.kernel.org/r/1466005756-15626-1-git-send-email-mark.rutland@arm.com
>> [2] http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutland@arm.com
>>
>> diff --git a/Makefile b/Makefile
>> index b409076..699d363 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -687,6 +687,8 @@ ifdef CONFIG_KCOV
>>      $(warning Cannot use CONFIG_KCOV: \
>>               -fsanitize-coverage=trace-pc is not supported by compiler)
>>      CFLAGS_KCOV =
>> +  else
>> +    KBUILD_CFLAGS += -DCC_HAVE_SANCOV_TRACE_PC
>>    endif
>>  endif
>>
>> diff --git a/kernel/kcov.c b/kernel/kcov.c
>> index a02f2dd..0a0b164 100644
>> --- a/kernel/kcov.c
>> +++ b/kernel/kcov.c
>> @@ -3,6 +3,7 @@
>>  #define DISABLE_BRANCH_PROFILING
>>  #include <linux/compiler.h>
>>  #include <linux/types.h>
>> +#include <linux/errno.h>
>>  #include <linux/file.h>
>>  #include <linux/fs.h>
>>  #include <linux/mm.h>
>> @@ -160,6 +161,14 @@ static int kcov_open(struct inode *inode, struct file *filep)
>>  {
>>         struct kcov *kcov;
>>
>> +#ifndef CC_HAVE_SANCOV_TRACE_PC
>> +       /*
>> +        * CONFIG_KCOV was selected, but the compiler does not support the
>> +        * options KCOV requires.
>> +        */
>> +       return -ENOTSUPP;
>> +#endif /* CC_HAVE_SANCOV_TRACE_PC */
>> +
>>         kcov = kzalloc(sizeof(*kcov), GFP_KERNEL);
>>         if (!kcov)
>>                 return -ENOMEM;
>> --
>> 1.9.1
>>
>
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg