Re: [PATCHv3] kcov: reject open when kernel not instrumented
From: Alexander Potapenko
Date: Fri Jun 17 2016 - 11:46:48 EST
On Fri, Jun 17, 2016 at 5:42 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland <mark.rutland@xxxxxxx> 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@xxxxxxx>
>> Cc: Alexander Potapenko <glider@xxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
>> Cc: James Morse <james.morse@xxxxxxx>
>> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
>> Cc: Michal Marek <mmarek@xxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> ---
>> 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@xxxxxxx
>> [2] http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutland@xxxxxxx
>>
>> 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