Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 5:48 PM, Mark Rutlandwrote: > On Fri, Jun 17, 2016 at 08:42:28AM -0700, Kees Cook wrote: >> On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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 would also be happy with that, so it's up to Alexander and Dmitry. > >> 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 >> >> If you wanted, the CONFIG_KCOV test could live under the same >> prepare-compiler-check target. > > Alexander, Dmitry, thoughts? I did it initially for KCOV. I just reported a warning, and then compiler errors on unknown flag. And it was submitted this way. But then Andrew did: http://www.spinics.net/lists/mm-commits/msg116008.html I've seen other "unbreak allmodconfig" patches. This issue does not affect my workflow, but it seems to be something that other people care about. I.e. you can't even test that code builds without a special compiler.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 5:48 PM, Mark Rutland wrote: > On Fri, Jun 17, 2016 at 08:42:28AM -0700, Kees Cook wrote: >> On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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 would also be happy with that, so it's up to Alexander and Dmitry. > >> 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 >> >> If you wanted, the CONFIG_KCOV test could live under the same >> prepare-compiler-check target. > > Alexander, Dmitry, thoughts? I did it initially for KCOV. I just reported a warning, and then compiler errors on unknown flag. And it was submitted this way. But then Andrew did: http://www.spinics.net/lists/mm-commits/msg116008.html I've seen other "unbreak allmodconfig" patches. This issue does not affect my workflow, but it seems to be something that other people care about. I.e. you can't even test that code builds without a special compiler.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 05:46:38PM +0200, Alexander Potapenko wrote: > On Fri, Jun 17, 2016 at 5:42 PM, Kees Cookwrote: > > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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=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. My concern was that it was easy to miss the build-time warning, and having the kcov file visible and accessible didn't make clear to the user that the feature won't actually work. I'm happy for a failed kcov_open or anything stronger than that (e.g. the kcov file not bring present, or the build being aborted). Thanks, Mark.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 05:46:38PM +0200, Alexander Potapenko wrote: > On Fri, Jun 17, 2016 at 5:42 PM, Kees Cook wrote: > > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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=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. My concern was that it was easy to miss the build-time warning, and having the kcov file visible and accessible didn't make clear to the user that the feature won't actually work. I'm happy for a failed kcov_open or anything stronger than that (e.g. the kcov file not bring present, or the build being aborted). Thanks, Mark.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 5:42 PM, Kees Cookwrote: > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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=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 >> Cc: Alexander Potapenko >> Cc: Andrew Morton >> Cc: Dmitry Vyukov >> Cc: James Morse >> Cc: Kees Cook >> Cc: Michal Marek >> Cc: linux-kernel@vger.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.rutl...@arm.com >> [2] >> http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 >> #include >> +#include >> #include >> #include >> #include >> @@ -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
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 5:42 PM, Kees Cook wrote: > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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=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 >> Cc: Alexander Potapenko >> Cc: Andrew Morton >> Cc: Dmitry Vyukov >> Cc: James Morse >> Cc: Kees Cook >> Cc: Michal Marek >> Cc: linux-kernel@vger.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.rutl...@arm.com >> [2] >> http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 >> #include >> +#include >> #include >> #include >> #include >> @@ -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
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 08:42:28AM -0700, Kees Cook wrote: > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutlandwrote: > > 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 would also be happy with that, so it's up to Alexander and Dmitry. > 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 > > If you wanted, the CONFIG_KCOV test could live under the same > prepare-compiler-check target. Alexander, Dmitry, thoughts? Thanks, Mark.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 08:42:28AM -0700, Kees Cook wrote: > On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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 would also be happy with that, so it's up to Alexander and Dmitry. > 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 > > If you wanted, the CONFIG_KCOV test could live under the same > prepare-compiler-check target. Alexander, Dmitry, thoughts? Thanks, Mark.
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutlandwrote: > 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 If you wanted, the CONFIG_KCOV test could live under the same prepare-compiler-check target. -Kees > > 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 > Cc: Alexander Potapenko > Cc: Andrew Morton > Cc: Dmitry Vyukov > Cc: James Morse > Cc: Kees Cook > Cc: Michal Marek > Cc: linux-kernel@vger.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.rutl...@arm.com > [2] > http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 > #include > +#include > #include > #include > #include > @@ -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
Re: [PATCHv3] kcov: reject open when kernel not instrumented
On Fri, Jun 17, 2016 at 2:39 AM, Mark Rutland 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=600c1bd5f8647a8470dc2fc5a8697e3eafb5fd52 If you wanted, the CONFIG_KCOV test could live under the same prepare-compiler-check target. -Kees > > 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 > Cc: Alexander Potapenko > Cc: Andrew Morton > Cc: Dmitry Vyukov > Cc: James Morse > Cc: Kees Cook > Cc: Michal Marek > Cc: linux-kernel@vger.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.rutl...@arm.com > [2] > http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 > #include > +#include > #include > #include > #include > @@ -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
[PATCHv3] kcov: reject open when kernel not instrumented
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. 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 RutlandCc: Alexander Potapenko Cc: Andrew Morton Cc: Dmitry Vyukov Cc: James Morse Cc: Kees Cook Cc: Michal Marek Cc: linux-kernel@vger.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.rutl...@arm.com [2] http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 #include +#include #include #include #include @@ -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
[PATCHv3] kcov: reject open when kernel not instrumented
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. 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 Cc: Alexander Potapenko Cc: Andrew Morton Cc: Dmitry Vyukov Cc: James Morse Cc: Kees Cook Cc: Michal Marek Cc: linux-kernel@vger.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.rutl...@arm.com [2] http://lkml.kernel.org/r/1466010285-2772-1-git-send-email-mark.rutl...@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 #include +#include #include #include #include @@ -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