From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753950AbcFPK2f (ORCPT ); Thu, 16 Jun 2016 06:28:35 -0400 Received: from foss.arm.com ([217.140.101.70]:42762 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751681AbcFPK2d (ORCPT ); Thu, 16 Jun 2016 06:28:33 -0400 Date: Thu, 16 Jun 2016 11:28:10 +0100 From: Mark Rutland To: James Morse Cc: linux-kernel@vger.kernel.org, Alexander Potapenko , Andrew Morton , Dmitry Vyukov , Kees Cook , Michal Marek Subject: Re: [PATCHv2] kcov: reject open when kernel not instrumented Message-ID: <20160616102810.GB4184@leverpostej> References: <1466010285-2772-1-git-send-email-mark.rutland@arm.com> <57627AE1.9020102@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57627AE1.9020102@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 16, 2016 at 11:09:37AM +0100, James Morse wrote: > Hi Mark, > > On 15/06/16 18:04, 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. > > > > 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. > > > diff --git a/Makefile b/Makefile > > index 0f70de6..3785a63 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -369,7 +369,7 @@ LDFLAGS_MODULE = > > CFLAGS_KERNEL = > > AFLAGS_KERNEL = > > CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized > > -CFLAGS_KCOV = -fsanitize-coverage=trace-pc > > +CFLAGS_KCOV = -fsanitize-coverage=trace-pc -DCC_HAVE_SANCOV_TRACE_PC > > > > > > # Use USERINCLUDE when you must reference the UAPI directories only. > > 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 > > I don't think this will work as kernel/kcov.c is listed in the Makefile as: > > # Don't self-instrument. > > KCOV_INSTRUMENT_kcov.o := n > > > This will cause the build machinery in scripts/Makefile.lib to not give > kernel/kcov.c the CFLAGS_KCOV contents: > > ifeq ($(CONFIG_KCOV),y) > > _c_flags += $(if $(patsubst n%,, \ > > $(KCOV_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)y), \ > > $(CFLAGS_KCOV)) > > endif > > ... so kernel/kcov.c will never see anything in CFLAGS_KCOV ... Good spot! Evidently I was trying to be overly clever here; my bad. > An alternative would be to add the flag to the compiler test that generates the > 'not supported' warning, but it needs to go in another CFLAGS variable. > Something like: > > -------------------%<------------------- > diff --git 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 > -------------------%<------------------- I'll fold the above in for v3. Thanks, Mark.