diff mbox series

[v5,3/6] kasan: Add report for async mode

Message ID 20210121163943.9889-4-vincenzo.frascino@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: ARMv8.5-A: MTE: Add async mode support | expand

Commit Message

Vincenzo Frascino Jan. 21, 2021, 4:39 p.m. UTC 
KASAN provides an asynchronous mode of execution.

Add reporting functionality for this mode.

Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 include/linux/kasan.h |  2 ++
 mm/kasan/report.c     | 11 +++++++++++
 2 files changed, 13 insertions(+)

Comments

Andrey Konovalov Jan. 21, 2021, 5:36 p.m. UTC | #1 
On Thu, Jan 21, 2021 at 5:39 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> KASAN provides an asynchronous mode of execution.
>
> Add reporting functionality for this mode.
>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  include/linux/kasan.h |  2 ++
>  mm/kasan/report.c     | 11 +++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index bb862d1f0e15..b0a1d9dfa85c 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -351,6 +351,8 @@ static inline void *kasan_reset_tag(const void *addr)
>  bool kasan_report(unsigned long addr, size_t size,
>                 bool is_write, unsigned long ip);
>
> +void kasan_report_async(void);
> +
>  #else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>
>  static inline void *kasan_reset_tag(const void *addr)
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 234f35a84f19..2fd6845a95e9 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -358,6 +358,17 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>         end_report(&flags);
>  }
>
> +void kasan_report_async(void)
> +{
> +       unsigned long flags;
> +
> +       start_report(&flags);
> +       pr_err("BUG: KASAN: invalid-access\n");
> +       pr_err("Asynchronous mode enabled: no access details available\n");
> +       dump_stack();
> +       end_report(&flags);
> +}
> +
>  static void __kasan_report(unsigned long addr, size_t size, bool is_write,
>                                 unsigned long ip)
>  {
> --
> 2.30.0
>

Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

FTR: this will conflict with the Alex's patch:

https://lore.kernel.org/linux-api/20210121131915.1331302-1-glider@google.com/T/#m8872c56af85babfc08784e2b2fcd5cc1c0c73859
kernel test robot Jan. 22, 2021, 2:45 a.m. UTC | #2 
Hi Vincenzo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20210121]
[cannot apply to arm64/for-next/core arm/for-next soc/for-next xlnx/master kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc4 v5.11-rc3 v5.11-rc2 v5.11-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210122-004631
base:    bc085f8fc88fc16796c9f2364e2bfb3fef305cad
config: riscv-randconfig-r003-20210122 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bd3a387ee76f58caa0d7901f3f84e9bb3d006f27)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/5d51fa880ab55b639b377b24bfe0b8ef6560c14c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210122-004631
        git checkout 5d51fa880ab55b639b377b24bfe0b8ef6560c14c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> mm/kasan/report.c:361:6: warning: no previous prototype for function 'kasan_report_async' [-Wmissing-prototypes]
   void kasan_report_async(void)
        ^
   mm/kasan/report.c:361:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void kasan_report_async(void)
   ^
   static 
   1 warning generated.


vim +/kasan_report_async +361 mm/kasan/report.c

   360	
 > 361	void kasan_report_async(void)
   362	{
   363		unsigned long flags;
   364	
   365		start_report(&flags);
   366		pr_err("BUG: KASAN: invalid-access\n");
   367		pr_err("Asynchronous mode enabled: no access details available\n");
   368		dump_stack();
   369		end_report(&flags);
   370	}
   371	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Jan. 22, 2021, 2:46 a.m. UTC | #3 
Hi Vincenzo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20210121]
[cannot apply to arm64/for-next/core arm/for-next soc/for-next xlnx/master kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc4 v5.11-rc3 v5.11-rc2 v5.11-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210122-004631
base:    bc085f8fc88fc16796c9f2364e2bfb3fef305cad
config: x86_64-randconfig-s022-20210122 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-208-g46a52ca4-dirty
        # https://github.com/0day-ci/linux/commit/5d51fa880ab55b639b377b24bfe0b8ef6560c14c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210122-004631
        git checkout 5d51fa880ab55b639b377b24bfe0b8ef6560c14c
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> mm/kasan/report.c:361:6: warning: no previous prototype for 'kasan_report_async' [-Wmissing-prototypes]
     361 | void kasan_report_async(void)
         |      ^~~~~~~~~~~~~~~~~~


vim +/kasan_report_async +361 mm/kasan/report.c

   360	
 > 361	void kasan_report_async(void)
   362	{
   363		unsigned long flags;
   364	
   365		start_report(&flags);
   366		pr_err("BUG: KASAN: invalid-access\n");
   367		pr_err("Asynchronous mode enabled: no access details available\n");
   368		dump_stack();
   369		end_report(&flags);
   370	}
   371	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Catalin Marinas Jan. 22, 2021, 1:19 p.m. UTC | #4 
On Thu, Jan 21, 2021 at 04:39:40PM +0000, Vincenzo Frascino wrote:
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index bb862d1f0e15..b0a1d9dfa85c 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -351,6 +351,8 @@ static inline void *kasan_reset_tag(const void *addr)
>  bool kasan_report(unsigned long addr, size_t size,
>  		bool is_write, unsigned long ip);
>  
> +void kasan_report_async(void);
> +
>  #else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>  
>  static inline void *kasan_reset_tag(const void *addr)
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 234f35a84f19..2fd6845a95e9 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -358,6 +358,17 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>  	end_report(&flags);
>  }
>  
> +void kasan_report_async(void)
> +{
> +	unsigned long flags;
> +
> +	start_report(&flags);
> +	pr_err("BUG: KASAN: invalid-access\n");
> +	pr_err("Asynchronous mode enabled: no access details available\n");
> +	dump_stack();
> +	end_report(&flags);
> +}

I think the kernel test robot complains that with KASAN_SW_TAGS and
HW_TAGS disabled, the kasan_report_async() prototype is no longer
visible but you still have the non-static function definition here. So
either move kasan_report_async() out of this #ifdef or add the #ifdef
around the function definition.

It looks like the original kasan_report() prototype is declared in two
places (second one in mm/kasan/kasan.h). I'd remove the latter and try
to have a consistent approach for kasan_report() and
kasan_report_async().
Vincenzo Frascino Jan. 22, 2021, 1:27 p.m. UTC | #5 
On 1/22/21 1:19 PM, Catalin Marinas wrote:
> On Thu, Jan 21, 2021 at 04:39:40PM +0000, Vincenzo Frascino wrote:
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index bb862d1f0e15..b0a1d9dfa85c 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -351,6 +351,8 @@ static inline void *kasan_reset_tag(const void *addr)
>>  bool kasan_report(unsigned long addr, size_t size,
>>  		bool is_write, unsigned long ip);
>>  
>> +void kasan_report_async(void);
>> +
>>  #else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>>  
>>  static inline void *kasan_reset_tag(const void *addr)
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index 234f35a84f19..2fd6845a95e9 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -358,6 +358,17 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
>>  	end_report(&flags);
>>  }
>>  
>> +void kasan_report_async(void)
>> +{
>> +	unsigned long flags;
>> +
>> +	start_report(&flags);
>> +	pr_err("BUG: KASAN: invalid-access\n");
>> +	pr_err("Asynchronous mode enabled: no access details available\n");
>> +	dump_stack();
>> +	end_report(&flags);
>> +}
> 
> I think the kernel test robot complains that with KASAN_SW_TAGS and
> HW_TAGS disabled, the kasan_report_async() prototype is no longer
> visible but you still have the non-static function definition here. So
> either move kasan_report_async() out of this #ifdef or add the #ifdef
> around the function definition.
>

I think adding #ifdef around the function would be the best way in this case,
for consistency with the header.

> It looks like the original kasan_report() prototype is declared in two
> places (second one in mm/kasan/kasan.h). I'd remove the latter and try
> to have a consistent approach for kasan_report() and
> kasan_report_async().
> 

Ok, I will remove it.
Vincenzo Frascino Jan. 22, 2021, 1:38 p.m. UTC | #6 
On 1/22/21 1:27 PM, Vincenzo Frascino wrote:
>> It looks like the original kasan_report() prototype is declared in two
>> places (second one in mm/kasan/kasan.h). I'd remove the latter and try
>> to have a consistent approach for kasan_report() and
>> kasan_report_async().
>>
> Ok, I will remove it.

I just realized that the internal interface exposes the kasan_report() interface
for the GENERIC KASAN implementation. If I remove it that does not work anymore:

/data1/Projects/LinuxKernel/linux-mte/mm/kasan/common.c: In function
‘__kasan_check_byte’:
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/common.c:503:17: error: implicit
declaration of function ‘kasan_report’ [-Werror=implicit-function-declaration]
  503 |                 kasan_report((unsigned long)address, 1, false, ip);
      |                 ^~~~~~~~~~~~
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/generic.c: In function
‘check_region_inline’:
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/generic.c:170:25: error: implicit
declaration of function ‘kasan_report’ [-Werror=implicit-function-declaration]
  170 |                 return !kasan_report(addr, size, write, ret_ip);
      |                         ^~~~~~~~~~~~
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/report_generic.c: In function
‘__asan_report_load1_noabort’:
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/report_generic.c:295:9: error:
implicit declaration of function ‘kasan_report’
[-Werror=implicit-function-declaration]
  295 |         kasan_report(addr, size, false, _RET_IP_);        \
      |         ^~~~~~~~~~~~
/data1/Projects/LinuxKernel/linux-mte/mm/kasan/report_generic.c:306:1: note: in
expansion of macro ‘DEFINE_ASAN_REPORT_LOAD’
  306 | DEFINE_ASAN_REPORT_LOAD(1);

To do that cleanly few things need to be shuffled around, Andrey or I can take
care of it but if you agree we shall look into this after -rc1.

Thanks!
diff mbox series

Patch

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index bb862d1f0e15..b0a1d9dfa85c 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -351,6 +351,8 @@  static inline void *kasan_reset_tag(const void *addr)
 bool kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 
+void kasan_report_async(void);
+
 #else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
 
 static inline void *kasan_reset_tag(const void *addr)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 234f35a84f19..2fd6845a95e9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -358,6 +358,17 @@  void kasan_report_invalid_free(void *object, unsigned long ip)
 	end_report(&flags);
 }
 
+void kasan_report_async(void)
+{
+	unsigned long flags;
+
+	start_report(&flags);
+	pr_err("BUG: KASAN: invalid-access\n");
+	pr_err("Asynchronous mode enabled: no access details available\n");
+	dump_stack();
+	end_report(&flags);
+}
+
 static void __kasan_report(unsigned long addr, size_t size, bool is_write,
 				unsigned long ip)
 {