LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 6/7] powerpc/eeh: Allow disabling recovery
From: Sam Bobroff @ 2019-02-15  5:58 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20190215004817.19961-6-oohall@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

On Fri, Feb 15, 2019 at 11:48:16AM +1100, Oliver O'Halloran wrote:
> Currently when we detect an error we automatically invoke the EEH recovery
> handler. This can be annoying when debugging EEH problems, or when working
> on EEH itself so this patch adds a debugfs knob that will prevent a
> recovery event from being queued up when an issue is detected.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
>  arch/powerpc/include/asm/eeh.h  |  1 +
>  arch/powerpc/kernel/eeh.c       | 10 ++++++++++
>  arch/powerpc/kernel/eeh_event.c |  9 +++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 478f199d5663..810e05273ad3 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -220,6 +220,7 @@ struct eeh_ops {
>  
>  extern int eeh_subsystem_flags;
>  extern u32 eeh_max_freezes;
> +extern bool eeh_debugfs_no_recover;
>  extern struct eeh_ops *eeh_ops;
>  extern raw_spinlock_t confirm_error_lock;
>  
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 82d22c671c0e..9f20099ce2d9 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -111,6 +111,13 @@ EXPORT_SYMBOL(eeh_subsystem_flags);
>   */
>  u32 eeh_max_freezes = 5;
>  
> +/*
> + * Controls whether a recovery event should be scheduled when an
> + * isolated device is discovered. This is only really useful for
> + * debugging problems with the EEH core.
> + */
> +bool eeh_debugfs_no_recover;
> +
>  /* Platform dependent EEH operations */
>  struct eeh_ops *eeh_ops = NULL;
>  
> @@ -1810,6 +1817,9 @@ static int __init eeh_init_proc(void)
>  					   &eeh_enable_dbgfs_ops);
>  		debugfs_create_u32("eeh_max_freezes", 0600,
>  				powerpc_debugfs_root, &eeh_max_freezes);
> +		debugfs_create_bool("eeh_disable_recovery", 0600,
> +				powerpc_debugfs_root,
> +				&eeh_debugfs_no_recover);
>  		eeh_cache_debugfs_init();
>  #endif
>  	}
> diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
> index 227e57f980df..19837798bb1d 100644
> --- a/arch/powerpc/kernel/eeh_event.c
> +++ b/arch/powerpc/kernel/eeh_event.c
> @@ -126,6 +126,15 @@ int eeh_send_failure_event(struct eeh_pe *pe)
>  	unsigned long flags;
>  	struct eeh_event *event;
>  
> +	/*
> +	 * If we've manually supressed recovery events via debugfs
> +	 * then just drop it on the floor.
> +	 */
> +	if (eeh_debugfs_no_recover) {
> +		pr_err("EEH: Event dropped due to no_recover setting\n");
> +		return 0;
> +	}
> +

I think it might be clearer if you did the 'no recovery' test at the
call sites (I think there are only a few), instead of inside the
function (and then the next patch wouldn't need to add a wrapper).

>  	event = kzalloc(sizeof(*event), GFP_ATOMIC);
>  	if (!event) {
>  		pr_err("EEH: out of memory, event not handled\n");
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] powerpc/ptrace: Simplify vr_get/set() to avoid GCC warning
From: Michael Ellerman @ 2019-02-15  6:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: malat, mroos

GCC 8 warns about the logic in vr_get/set(), which with -Werror breaks
the build:

  In function ‘user_regset_copyin’,
      inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:628:9:
  include/linux/regset.h:295:4: error: ‘memcpy’ offset [-527, -529] is
  out of the bounds [0, 16] of object ‘vrsave’ with type ‘union
  <anonymous>’ [-Werror=array-bounds]
  arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
  arch/powerpc/kernel/ptrace.c:623:5: note: ‘vrsave’ declared here
     } vrsave;

This has been identified as a regression in GCC, see GCC bug 88273.

However we can avoid the warning and also simplify the logic and make
it more robust.

Currently we pass -1 as end_pos to user_regset_copyout(). This says
"copy up to the end of the regset".

The definition of the regset is:
	[REGSET_VMX] = {
		.core_note_type = NT_PPC_VMX, .n = 34,
		.size = sizeof(vector128), .align = sizeof(vector128),
		.active = vr_active, .get = vr_get, .set = vr_set
	},

The end is calculated as (n * size), ie. 34 * sizeof(vector128).

In vr_get/set() we pass start_pos as 33 * sizeof(vector128), meaning
we can copy up to sizeof(vector128) into/out-of vrsave.

The on-stack vrsave is defined as:
  union {
	  elf_vrreg_t reg;
	  u32 word;
  } vrsave;

And elf_vrreg_t is:
  typedef __vector128 elf_vrreg_t;

So there is no bug, but we rely on all those sizes lining up,
otherwise we would have a kernel stack exposure/overwrite on our
hands.

Rather than relying on that we can pass an explict end_pos based on
the sizeof(vrsave). The result should be exactly the same but it's
more obviously not over-reading/writing the stack and it avoids the
compiler warning.

Reported-by: Meelis Roos <mroos@linux.ee>
Reported-by: Mathieu Malaterre <malat@debian.org>
Cc: stable@vger.kernel.org
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/ptrace.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 7535f89e08cd..d9ac7d94656e 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -567,6 +567,7 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
 		/*
 		 * Copy out only the low-order word of vrsave.
 		 */
+		int start, end;
 		union {
 			elf_vrreg_t reg;
 			u32 word;
@@ -575,8 +576,10 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
 
 		vrsave.word = target->thread.vrsave;
 
+		start = 33 * sizeof(vector128);
+		end = start + sizeof(vrsave);
 		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
-					  33 * sizeof(vector128), -1);
+					  start, end);
 	}
 
 	return ret;
@@ -614,6 +617,7 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
 		/*
 		 * We use only the first word of vrsave.
 		 */
+		int start, end;
 		union {
 			elf_vrreg_t reg;
 			u32 word;
@@ -622,8 +626,10 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
 
 		vrsave.word = target->thread.vrsave;
 
+		start = 33 * sizeof(vector128);
+		end = start + sizeof(vrsave);
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
-					 33 * sizeof(vector128), -1);
+					 start, end);
 		if (!ret)
 			target->thread.vrsave = vrsave.word;
 	}
-- 
2.20.1


^ permalink raw reply related

* Re: [RFC PATCH 2/5] kasan: allow architectures to manage the memory-to-shadow mapping
From: Dmitry Vyukov @ 2019-02-15  6:35 UTC (permalink / raw)
  To: Daniel Axtens; +Cc: Aneesh Kumar K.V, linuxppc-dev, kasan-dev
In-Reply-To: <20190215000441.14323-3-dja@axtens.net>

On Fri, Feb 15, 2019 at 1:05 AM Daniel Axtens <dja@axtens.net> wrote:
>
> Currently, shadow addresses are always addr >> shift + offset.
> However, for powerpc, the virtual address space is fragmented in
> ways that make this simple scheme impractical.
>
> Allow architectures to override:
>  - kasan_shadow_to_mem
>  - kasan_mem_to_shadow
>  - addr_has_shadow
>
> Rename addr_has_shadow to kasan_addr_has_shadow as if it is
> overridden it will be available in more places, increasing the
> risk of collisions.
>
> If architectures do not #define their own versions, the generic
> code will continue to run as usual.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

> ---
>  include/linux/kasan.h     | 2 ++
>  mm/kasan/generic.c        | 2 +-
>  mm/kasan/generic_report.c | 2 +-
>  mm/kasan/kasan.h          | 6 +++++-
>  mm/kasan/report.c         | 6 +++---
>  mm/kasan/tags.c           | 2 +-
>  6 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index b40ea104dd36..f6261840f94c 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -23,11 +23,13 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>  int kasan_populate_early_shadow(const void *shadow_start,
>                                 const void *shadow_end);
>
> +#ifndef kasan_mem_to_shadow
>  static inline void *kasan_mem_to_shadow(const void *addr)
>  {
>         return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>                 + KASAN_SHADOW_OFFSET;
>  }
> +#endif
>
>  /* Enable reporting bugs after kasan_disable_current() */
>  extern void kasan_enable_current(void);
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index ffc64a9a97a5..bafa2f986660 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -173,7 +173,7 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
>         if (unlikely(size == 0))
>                 return;
>
> -       if (unlikely(!addr_has_shadow((void *)addr))) {
> +       if (unlikely(!kasan_addr_has_shadow((void *)addr))) {
>                 kasan_report(addr, size, write, ret_ip);
>                 return;
>         }
> diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c
> index 5e12035888f2..854f4de1fe10 100644
> --- a/mm/kasan/generic_report.c
> +++ b/mm/kasan/generic_report.c
> @@ -110,7 +110,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)
>
>  const char *get_bug_type(struct kasan_access_info *info)
>  {
> -       if (addr_has_shadow(info->access_addr))
> +       if (kasan_addr_has_shadow(info->access_addr))
>                 return get_shadow_bug_type(info);
>         return get_wild_bug_type(info);
>  }
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index ea51b2d898ec..57ec24cf7bd1 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -111,16 +111,20 @@ struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
>  struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
>                                         const void *object);
>
> +#ifndef kasan_shadow_to_mem
>  static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
>  {
>         return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET)
>                 << KASAN_SHADOW_SCALE_SHIFT);
>  }
> +#endif
>
> -static inline bool addr_has_shadow(const void *addr)
> +#ifndef kasan_addr_has_shadow
> +static inline bool kasan_addr_has_shadow(const void *addr)
>  {
>         return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
>  }
> +#endif
>
>  void kasan_poison_shadow(const void *address, size_t size, u8 value);
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index ca9418fe9232..bc3355ee2dd0 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -298,7 +298,7 @@ void kasan_report(unsigned long addr, size_t size,
>         untagged_addr = reset_tag(tagged_addr);
>
>         info.access_addr = tagged_addr;
> -       if (addr_has_shadow(untagged_addr))
> +       if (kasan_addr_has_shadow(untagged_addr))
>                 info.first_bad_addr = find_first_bad_addr(tagged_addr, size);
>         else
>                 info.first_bad_addr = untagged_addr;
> @@ -309,11 +309,11 @@ void kasan_report(unsigned long addr, size_t size,
>         start_report(&flags);
>
>         print_error_description(&info);
> -       if (addr_has_shadow(untagged_addr))
> +       if (kasan_addr_has_shadow(untagged_addr))
>                 print_tags(get_tag(tagged_addr), info.first_bad_addr);
>         pr_err("\n");
>
> -       if (addr_has_shadow(untagged_addr)) {
> +       if (kasan_addr_has_shadow(untagged_addr)) {
>                 print_address_description(untagged_addr);
>                 pr_err("\n");
>                 print_shadow_for_address(info.first_bad_addr);
> diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> index bc759f8f1c67..cdefd0fe1f5d 100644
> --- a/mm/kasan/tags.c
> +++ b/mm/kasan/tags.c
> @@ -109,7 +109,7 @@ void check_memory_region(unsigned long addr, size_t size, bool write,
>                 return;
>
>         untagged_addr = reset_tag((const void *)addr);
> -       if (unlikely(!addr_has_shadow(untagged_addr))) {
> +       if (unlikely(!kasan_addr_has_shadow(untagged_addr))) {
>                 kasan_report(addr, size, write, ret_ip);
>                 return;
>         }
> --
> 2.19.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190215000441.14323-3-dja%40axtens.net.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH 06/11] lockdep: consolidate the LOCKDEP_SUPPORT symbol
From: Masahiro Yamada @ 2019-02-15  7:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, Greentime Hu, linux-riscv,
	Vincent Chen, linuxppc-dev
In-Reply-To: <20190213174005.28785-7-hch@lst.de>

+CC Greentime Hu
+CC Vincent Chen


On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Add one definition to lib/Kconfig.debug and let the architectures
> select if it supported.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>


This changes the behavior of nds32.

So please leave a note in the commit description.
(Ideally, we want Ack of a NDS32 maintainer)

Also, please do compile test for NDS32.


-------------------------
This commit changes CONFIG_LOCKDEP_SUPPORT to y for nds32.

NDS32 selects LOCKEP_SUPPORT, but there was previously
no config entry of LOCKDEP_SUPPORT in nds32.

It is now available in lib/Kconfig.debug,
so this commit enables LOCKDEP_SUPPORT for nds32.
-------------------------






--
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 11/11] s390: don't redefined the HAS_IOMEM symbol
From: Masahiro Yamada @ 2019-02-15  7:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-12-hch@lst.de>

On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Rely on the common defintion instead.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/s390/Kconfig | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 9a25e19364f5..0f62e33ffcb2 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -159,6 +159,7 @@ config S390
>         select MODULES_USE_ELF_RELA
>         select NEED_DMA_MAP_STATE       if PCI
>         select NEED_SG_DMA_LENGTH       if PCI
> +       select NO_IOMEM                 if !PCI


This does not work because NO_IOMEM is defined
in arch/um/Kconfig.


If you want to do this,
you must move NO_IOMEM  to lib/Kconfig.



>         select OLD_SIGACTION
>         select OLD_SIGSUSPEND3
>         select PCI_DOMAINS              if PCI
> @@ -708,9 +709,6 @@ config PCI_NR_FUNCTIONS
>
>  endif  # PCI
>
> -config HAS_IOMEM
> -       def_bool PCI
> -
>  config CHSC_SCH
>         def_tristate m
>         prompt "Support for CHSC subchannels"
> --
> 2.20.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 05/11] tracing: consolidate the TRACE_IRQFLAGS_SUPPORT symbol
From: Masahiro Yamada @ 2019-02-15  7:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, nios2-dev, Ley Foon Tan, linux-riscv,
	linuxppc-dev
In-Reply-To: <20190213174005.28785-6-hch@lst.de>

+CC: Ley Foon Tan <lftan@altera.com>
+CC: nios2-dev@lists.rocketboards.org


On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:

> diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
> index 715e6c09b4a5..3b50689007f5 100644
> --- a/arch/nios2/Kconfig
> +++ b/arch/nios2/Kconfig
> @@ -20,6 +20,7 @@ config NIOS2
>         select OF_EARLY_FLATTREE
>         select SOC_BUS
>         select SPARSE_IRQ
> +       select TRACE_IRQFLAGS_SUPPORT


This is a behavior change of nios2.

TRACE_IRQFLAGS_SUPPORT was previously 'n',
and this commit is changing it to 'y'.




Strangely, nios2 defines TRACE_IRQFLAGS_SUPPORT twice
with different default values.

In Kconfig, the first one becomes effective.

In this case, 'def_bool n' in arch/nios2/Kconfig is used.
'def_bool y' in arch/nios2/Kconfig.debug is dead code.


I think this select should be dropped
to keep the current behavior,
but I hope the NIOS2 maintainer will give us
some comments just in case.




>         select USB_ARCH_HAS_HCD if USB_SUPPORT
>         select CPU_NO_EFFICIENT_FFS
>         select ARCH_DISCARD_MEMBLOCK
> @@ -39,9 +40,6 @@ config NO_IOPORT_MAP
>  config FPU
>         def_bool n
>
> -config TRACE_IRQFLAGS_SUPPORT
> -       def_bool n
> -
>  menu "Kernel features"
>
>  source "kernel/Kconfig.hz"
> diff --git a/arch/nios2/Kconfig.debug b/arch/nios2/Kconfig.debug
> index f1da8a7b17ff..a8bc06e96ef5 100644
> --- a/arch/nios2/Kconfig.debug
> +++ b/arch/nios2/Kconfig.debug
> @@ -1,8 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -config TRACE_IRQFLAGS_SUPPORT
> -       def_bool y
> -
>  config EARLY_PRINTK
>         bool "Activate early kernel debugging"
>         default y

--
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 06/11] lockdep: consolidate the LOCKDEP_SUPPORT symbol
From: Greentime Hu @ 2019-02-15  7:58 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, linux-riscv, Vincent Chen,
	linuxppc-dev, Christoph Hellwig
In-Reply-To: <CAK7LNAQZm2TXO1gxif2LZQzwYae2AiqgZnNGJBxBjiPwoZLqig@mail.gmail.com>

Hi Yamada,

Masahiro Yamada <yamada.masahiro@socionext.com> 於 2019年2月15日 週五 下午3:17寫道:
>
> +CC Greentime Hu
> +CC Vincent Chen
>
>
> On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Add one definition to lib/Kconfig.debug and let the architectures
> > select if it supported.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
>
> This changes the behavior of nds32.
>
> So please leave a note in the commit description.
> (Ideally, we want Ack of a NDS32 maintainer)
>
> Also, please do compile test for NDS32.
>
>
> -------------------------
> This commit changes CONFIG_LOCKDEP_SUPPORT to y for nds32.
>
> NDS32 selects LOCKEP_SUPPORT, but there was previously
> no config entry of LOCKDEP_SUPPORT in nds32.
>
> It is now available in lib/Kconfig.debug,
> so this commit enables LOCKDEP_SUPPORT for nds32.
> -------------------------

Thank you to let us know this change.
I think it's ok to enable LOCKDEP_SUPPORT for nds32.
I just verified it in 5.0.0-rc1.

^ permalink raw reply

* Re: [PATCH 09/11] lib: consolidate the GENERIC_CSUM symbol
From: Masahiro Yamada @ 2019-02-15  8:03 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-10-hch@lst.de>

On Thu, Feb 14, 2019 at 2:41 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Add one definition to lib/Kconfig and let the architectures
> select if it supported.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>



> diff --git a/arch/unicore32/Kconfig b/arch/unicore32/Kconfig
> index 52b4d48e351a..9de1d983a99a 100644
> --- a/arch/unicore32/Kconfig
> +++ b/arch/unicore32/Kconfig
> @@ -29,9 +29,6 @@ config UNICORE32
>           designs licensed by PKUnity Ltd.
>           Please see web page at <http://www.pkunity.com/>.
>
> -config GENERIC_CSUM
> -       def_bool y
> -
>  config NO_IOPORT_MAP
>         bool
>



You missed to add 'select GENERIC_CSUM' for unicore32.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH] powerpc/ptrace: Simplify vr_get/set() to avoid GCC warning
From: Mathieu Malaterre @ 2019-02-15  8:09 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Meelis Roos
In-Reply-To: <20190215061400.20302-1-mpe@ellerman.id.au>

On Fri, Feb 15, 2019 at 7:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> GCC 8 warns about the logic in vr_get/set(), which with -Werror breaks
> the build:
>
>   In function ‘user_regset_copyin’,
>       inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:628:9:
>   include/linux/regset.h:295:4: error: ‘memcpy’ offset [-527, -529] is
>   out of the bounds [0, 16] of object ‘vrsave’ with type ‘union
>   <anonymous>’ [-Werror=array-bounds]
>   arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
>   arch/powerpc/kernel/ptrace.c:623:5: note: ‘vrsave’ declared here
>      } vrsave;
>
> This has been identified as a regression in GCC, see GCC bug 88273.

Good point, this does not seems this will be backported.

> However we can avoid the warning and also simplify the logic and make
> it more robust.
>
> Currently we pass -1 as end_pos to user_regset_copyout(). This says
> "copy up to the end of the regset".
>
> The definition of the regset is:
>         [REGSET_VMX] = {
>                 .core_note_type = NT_PPC_VMX, .n = 34,
>                 .size = sizeof(vector128), .align = sizeof(vector128),
>                 .active = vr_active, .get = vr_get, .set = vr_set
>         },
>
> The end is calculated as (n * size), ie. 34 * sizeof(vector128).
>
> In vr_get/set() we pass start_pos as 33 * sizeof(vector128), meaning
> we can copy up to sizeof(vector128) into/out-of vrsave.
>
> The on-stack vrsave is defined as:
>   union {
>           elf_vrreg_t reg;
>           u32 word;
>   } vrsave;
>
> And elf_vrreg_t is:
>   typedef __vector128 elf_vrreg_t;
>
> So there is no bug, but we rely on all those sizes lining up,
> otherwise we would have a kernel stack exposure/overwrite on our
> hands.
>
> Rather than relying on that we can pass an explict end_pos based on
> the sizeof(vrsave). The result should be exactly the same but it's
> more obviously not over-reading/writing the stack and it avoids the
> compiler warning.
>

maybe:

Link: https://lkml.org/lkml/2018/8/16/117

In any case the warning is now gone:

Tested-by: Mathieu Malaterre <malat@debian.org>

> Reported-by: Meelis Roos <mroos@linux.ee>
> Reported-by: Mathieu Malaterre <malat@debian.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/ptrace.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 7535f89e08cd..d9ac7d94656e 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -567,6 +567,7 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
>                 /*
>                  * Copy out only the low-order word of vrsave.
>                  */
> +               int start, end;
>                 union {
>                         elf_vrreg_t reg;
>                         u32 word;
> @@ -575,8 +576,10 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
>
>                 vrsave.word = target->thread.vrsave;
>
> +               start = 33 * sizeof(vector128);
> +               end = start + sizeof(vrsave);
>                 ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
> -                                         33 * sizeof(vector128), -1);
> +                                         start, end);
>         }
>
>         return ret;
> @@ -614,6 +617,7 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
>                 /*
>                  * We use only the first word of vrsave.
>                  */
> +               int start, end;
>                 union {
>                         elf_vrreg_t reg;
>                         u32 word;
> @@ -622,8 +626,10 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
>
>                 vrsave.word = target->thread.vrsave;
>
> +               start = 33 * sizeof(vector128);
> +               end = start + sizeof(vrsave);
>                 ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
> -                                        33 * sizeof(vector128), -1);
> +                                        start, end);
>                 if (!ret)
>                         target->thread.vrsave = vrsave.word;
>         }
> --
> 2.20.1
>

^ permalink raw reply

* Re: [PATCH] powerpc/ptrace: Add prototype for function pt_regs_check
From: Mathieu Malaterre @ 2019-02-15  8:11 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras, Oleg Nesterov, LKML
In-Reply-To: <20181208154624.6504-1-malat@debian.org>

On Sat, Dec 8, 2018 at 4:46 PM Mathieu Malaterre <malat@debian.org> wrote:
>
> `pt_regs_check` is a dummy function, its purpose is to break the build
> if struct pt_regs and struct user_pt_regs don't match.
>
> This function has no functionnal purpose, and will get eliminated at
> link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>
> This commit adds a prototype to fix warning at W=1:
>
>   arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for ‘pt_regs_check’ [-Werror=missing-prototypes]
>
> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  arch/powerpc/kernel/ptrace.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index a398999d0770..341c0060b4c8 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
>         user_enter();
>  }
>
> +void __init pt_regs_check(void);
> +/* dummy function, its purpose is to break the build if struct pt_regs and
> + * struct user_pt_regs don't match.
> + */

Another trick which seems to work with GCC is:

-void __init pt_regs_check(void)
+static inline void __init pt_regs_check(void)

>  void __init pt_regs_check(void)
>  {
>         BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
> --
> 2.19.2
>

^ permalink raw reply

* [PATCH] powerpc/mm: Handle mmap_min_addr correctly in get_unmapped_area callback
From: Aneesh Kumar K.V @ 2019-02-15  8:16 UTC (permalink / raw)
  To: npiggin, benh, paulus, mpe; +Cc: Aneesh Kumar K.V, Laurent Dufour, linuxppc-dev

After we ALIGN up the address we need to make sure we didn't overflow
and resulted in zero address. In that case, we need to make sure that
the returned address is greater than mmap_min_addr.

Also when doing top-down search the low_limit is not PAGE_SIZE but rather
max(PAGE_SIZE, mmap_min_addr). This handle cases in which mmap_min_addr >
PAGE_SIZE.

This fixes selftest va_128TBswitch --run-hugetlb reporting failures when
run as non root user for

mmap(-1, MAP_HUGETLB)
mmap(-1, MAP_HUGETLB)

We also avoid the first mmap(-1, MAP_HUGETLB) returning NULL address as mmap address
with this change

CC: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/mm/hugetlbpage-radix.c |  5 +++--
 arch/powerpc/mm/slice.c             | 10 ++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/mm/hugetlbpage-radix.c b/arch/powerpc/mm/hugetlbpage-radix.c
index 2486bee0f93e..97c7a39ebc00 100644
--- a/arch/powerpc/mm/hugetlbpage-radix.c
+++ b/arch/powerpc/mm/hugetlbpage-radix.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/mm.h>
 #include <linux/hugetlb.h>
+#include <linux/security.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
 #include <asm/cacheflush.h>
@@ -73,7 +74,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 	if (addr) {
 		addr = ALIGN(addr, huge_page_size(h));
 		vma = find_vma(mm, addr);
-		if (high_limit - len >= addr &&
+		if (high_limit - len >= addr && addr >= mmap_min_addr &&
 		    (!vma || addr + len <= vm_start_gap(vma)))
 			return addr;
 	}
@@ -83,7 +84,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 	 */
 	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
 	info.length = len;
-	info.low_limit = PAGE_SIZE;
+	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
 	info.high_limit = mm->mmap_base + (high_limit - DEFAULT_MAP_WINDOW);
 	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
 	info.align_offset = 0;
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 06898c13901d..aec91dbcdc0b 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -32,6 +32,7 @@
 #include <linux/export.h>
 #include <linux/hugetlb.h>
 #include <linux/sched/mm.h>
+#include <linux/security.h>
 #include <asm/mman.h>
 #include <asm/mmu.h>
 #include <asm/copro.h>
@@ -377,6 +378,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
 	int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
 	unsigned long addr, found, prev;
 	struct vm_unmapped_area_info info;
+	unsigned long min_addr = max(PAGE_SIZE, mmap_min_addr);
 
 	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
 	info.length = len;
@@ -393,7 +395,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
 	if (high_limit > DEFAULT_MAP_WINDOW)
 		addr += mm->context.slb_addr_limit - DEFAULT_MAP_WINDOW;
 
-	while (addr > PAGE_SIZE) {
+	while (addr > min_addr) {
 		info.high_limit = addr;
 		if (!slice_scan_available(addr - 1, available, 0, &addr))
 			continue;
@@ -405,8 +407,8 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
 		 * Check if we need to reduce the range, or if we can
 		 * extend it to cover the previous available slice.
 		 */
-		if (addr < PAGE_SIZE)
-			addr = PAGE_SIZE;
+		if (addr < min_addr)
+			addr = min_addr;
 		else if (slice_scan_available(addr - 1, available, 0, &prev)) {
 			addr = prev;
 			goto prev_slice;
@@ -528,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
 		addr = _ALIGN_UP(addr, page_size);
 		slice_dbg(" aligned addr=%lx\n", addr);
 		/* Ignore hint if it's too large or overlaps a VMA */
-		if (addr > high_limit - len ||
+		if (addr > high_limit - len || addr < mmap_min_addr ||
 		    !slice_area_is_free(mm, addr, len))
 			addr = 0;
 	}
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 01/11] powerpc: remove dead ifdefs in <asm/checksum.h>
From: Christophe Leroy @ 2019-02-15  8:17 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, linux-kbuild, linux-xtensa, linux-kernel,
	Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190214170510.GC32441@lst.de>



Le 14/02/2019 à 18:05, Christoph Hellwig a écrit :
> On Thu, Feb 14, 2019 at 09:26:19AM +0100, Christophe Leroy wrote:
>> Could you also remove the 'config GENERIC_CSUM' item in
>> arch/powerpc/Kconfig ?
> 
> All the separate declarations go away later in this series.
> 

I saw, but the purpose of the later patch is to replace arch defined 
GENERIC_CSUM by a common one that arches select. For the powerpc you are 
not in that case as the powerpc does not select GENERIC_CSUM.

So I really believe that all stale bits of remaining GENERIC_CSUM in 
powerpc should go away as a single dedicated patch, as a fix of commit 
d4fde568a34a ("powerpc/64: Use optimized checksum routines on 
little-endian")

Regarding the #ifdef __KERNEL__ , I think we should do a wide cleanup in 
arch/powerpc/include/asm, not only asm/checksum.h

Christophe

^ permalink raw reply

* Re: [PATCH] powerpc/ptrace: Add prototype for function pt_regs_check
From: Christophe Leroy @ 2019-02-15  8:21 UTC (permalink / raw)
  To: Mathieu Malaterre, Michael Ellerman
  Cc: Paul Mackerras, linuxppc-dev, Oleg Nesterov, LKML
In-Reply-To: <CA+7wUswJy0z_+aytawQv6+1pOVvH2z8SfOz1sxziHtgjqZ1F9g@mail.gmail.com>



Le 15/02/2019 à 09:11, Mathieu Malaterre a écrit :
> On Sat, Dec 8, 2018 at 4:46 PM Mathieu Malaterre <malat@debian.org> wrote:
>>
>> `pt_regs_check` is a dummy function, its purpose is to break the build
>> if struct pt_regs and struct user_pt_regs don't match.
>>
>> This function has no functionnal purpose, and will get eliminated at
>> link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>>
>> This commit adds a prototype to fix warning at W=1:
>>
>>    arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for ‘pt_regs_check’ [-Werror=missing-prototypes]
>>
>> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>   arch/powerpc/kernel/ptrace.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
>> index a398999d0770..341c0060b4c8 100644
>> --- a/arch/powerpc/kernel/ptrace.c
>> +++ b/arch/powerpc/kernel/ptrace.c
>> @@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
>>          user_enter();
>>   }
>>
>> +void __init pt_regs_check(void);
>> +/* dummy function, its purpose is to break the build if struct pt_regs and
>> + * struct user_pt_regs don't match.
>> + */
> 
> Another trick which seems to work with GCC is:
> 
> -void __init pt_regs_check(void)
> +static inline void __init pt_regs_check(void)

Does this really work ? Did you test to ensure that the BUILD_BUG_ON 
still detect mismatch between struct pt_regs and struct user_pt_regs ?

Christophe

> 
>>   void __init pt_regs_check(void)
>>   {
>>          BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
>> --
>> 2.19.2
>>

^ permalink raw reply

* Re: [RFC PATCH 1/5] kasan: do not open-code addr_has_shadow
From: Dmitry Vyukov @ 2019-02-15  8:21 UTC (permalink / raw)
  To: Andrew Donnellan; +Cc: Aneesh Kumar K.V, kasan-dev, linuxppc-dev, Daniel Axtens
In-Reply-To: <f155a38b-c6ab-7825-71e2-15709f9410f6@au1.ibm.com>

On Fri, Feb 15, 2019 at 1:12 AM Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
>
> On 15/2/19 11:04 am, Daniel Axtens wrote:
> > We have a couple of places checking for the existence of a shadow
> > mapping for an address by open-coding the inverse of the check in
> > addr_has_shadow.
> >
> > Replace the open-coded versions with the helper. This will be
> > needed in future to allow architectures to override the layout
> > of the shadow mapping.
> >
> > Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

> > ---
> >   mm/kasan/generic.c | 3 +--
> >   mm/kasan/tags.c    | 3 +--
> >   2 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> > index ccb6207276e3..ffc64a9a97a5 100644
> > --- a/mm/kasan/generic.c
> > +++ b/mm/kasan/generic.c
> > @@ -173,8 +173,7 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
> >       if (unlikely(size == 0))
> >               return;
> >
> > -     if (unlikely((void *)addr <
> > -             kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) {
> > +     if (unlikely(!addr_has_shadow((void *)addr))) {
> >               kasan_report(addr, size, write, ret_ip);
> >               return;
> >       }
> > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> > index 0777649e07c4..bc759f8f1c67 100644
> > --- a/mm/kasan/tags.c
> > +++ b/mm/kasan/tags.c
> > @@ -109,8 +109,7 @@ void check_memory_region(unsigned long addr, size_t size, bool write,
> >               return;
> >
> >       untagged_addr = reset_tag((const void *)addr);
> > -     if (unlikely(untagged_addr <
> > -                     kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) {
> > +     if (unlikely(!addr_has_shadow(untagged_addr))) {
> >               kasan_report(addr, size, write, ret_ip);
> >               return;
> >       }
> >
>
> --
> Andrew Donnellan              OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com  IBM Australia Limited
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/f155a38b-c6ab-7825-71e2-15709f9410f6%40au1.ibm.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* [PATCH] powerpc/mm: Convert slb presence warning check to WARN_ON_ONCE
From: Aneesh Kumar K.V @ 2019-02-15  8:24 UTC (permalink / raw)
  To: npiggin, benh, paulus, mpe; +Cc: Aneesh Kumar K.V, linuxppc-dev

We are hitting false positive in some case. Till we root cause
this, convert WARN_ON to WARN_ON_WONCE.

A sample stack dump looks like
NIP [c00000000007ac40] assert_slb_presence+0x90/0xa0
LR [c00000000007b270] slb_flush_and_restore_bolted+0x90/0xc0
Call Trace:
 arch_send_call_function_ipi_mask+0xcc/0x110 (unreliable)
 0xc000000f9f38f560
 slice_flush_segments+0x58/0xb0
 on_each_cpu+0x74/0xf0
 slice_get_unmapped_area+0x6d4/0x9e0
 hugetlb_get_unmapped_area+0x124/0x150
 get_unmapped_area+0xf0/0x1a0
 do_mmap+0x1a4/0x6b0
 vm_mmap_pgoff+0xbc/0x150
 ksys_mmap_pgoff+0x260/0x2f0
 sys_mmap+0x104/0x130
 system_call+0x5c/0x70

We are checking whether we were able to successfully insert
kernel stack SLB entries. If that is not case we will crash next.
So we are not losing much debug data.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/mm/slb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index bc3914d54e26..dca0cbd71b60 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -71,7 +71,7 @@ static void assert_slb_presence(bool present, unsigned long ea)
 
 	asm volatile(__PPC_SLBFEE_DOT(%0, %1) : "=r"(tmp) : "r"(ea) : "cr0");
 
-	WARN_ON(present == (tmp == 0));
+	WARN_ON_ONCE(present == (tmp == 0));
 #endif
 }
 
-- 
2.20.1


^ permalink raw reply related

* Re: [RFC PATCH 3/5] kasan: allow architectures to provide an outline readiness check
From: Dmitry Vyukov @ 2019-02-15  8:25 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: Aneesh Kumar K.V, kasan-dev, Aneesh Kumar K . V, linuxppc-dev
In-Reply-To: <20190215000441.14323-4-dja@axtens.net>

On Fri, Feb 15, 2019 at 1:05 AM Daniel Axtens <dja@axtens.net> wrote:
>
> In powerpc (as I understand it), we spend a lot of time in boot
> running in real mode before MMU paging is initalised. During
> this time we call a lot of generic code, including printk(). If
> we try to access the shadow region during this time, things fail.
>
> My attempts to move early init before the first printk have not
> been successful. (Both previous RFCs for ppc64 - by 2 different
> people - have needed this trick too!)
>
> So, allow architectures to define a check_return_arch_not_ready()
> hook that bails out of check_memory_region_inline() unless the
> arch has done all of the init.
>
> Link: https://lore.kernel.org/patchwork/patch/592820/ # ppc64 hash series
> Link: https://patchwork.ozlabs.org/patch/795211/      # ppc radix series
> Originally-by: Balbir Singh <bsingharora@gmail.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  include/linux/kasan.h | 4 ++++
>  mm/kasan/generic.c    | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index f6261840f94c..83edc5e2b6a0 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -14,6 +14,10 @@ struct task_struct;
>  #include <asm/kasan.h>
>  #include <asm/pgtable.h>
>
> +#ifndef check_return_arch_not_ready
> +#define check_return_arch_not_ready()  do { } while (0)
> +#endif

Please do a bool-returning function. There is no need for
macro-super-powers here and normal C should be the default choice in
such cases.
It will be inlined and an empty impl will dissolve just as the macro.

>  extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
>  extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE];
>  extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index bafa2f986660..4c18bbd09a20 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -170,6 +170,8 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
>                                                 size_t size, bool write,
>                                                 unsigned long ret_ip)
>  {
> +       check_return_arch_not_ready();
> +
>         if (unlikely(size == 0))
>                 return;
>
> --
> 2.19.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190215000441.14323-4-dja%40axtens.net.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [RFC PATCH 5/5] powerpc: KASAN for 64bit Book3E
From: Dmitry Vyukov @ 2019-02-15  8:28 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: Aneesh Kumar K.V, kasan-dev, Aneesh Kumar K . V, linuxppc-dev
In-Reply-To: <20190215000441.14323-6-dja@axtens.net>

On Fri, Feb 15, 2019 at 1:05 AM Daniel Axtens <dja@axtens.net> wrote:
>
> Wire up KASAN. Only outline instrumentation is supported.
>
> The KASAN shadow area is mapped into vmemmap space:
> 0x8000 0400 0000 0000 to 0x8000 0600 0000 0000.
> To do this we require that vmemmap be disabled. (This is the default
> in the kernel config that QorIQ provides for the machine in their
> SDK anyway - they use flat memory.)
>
> Only the kernel linear mapping (0xc000...) is checked. The vmalloc and
> ioremap areas (also in 0x800...) are all mapped to a zero page. As
> with the Book3S hash series, this requires overriding the memory <->
> shadow mapping.
>
> Also, as with both previous 64-bit series, early instrumentation is not
> supported.  It would allow us to drop the check_return_arch_not_ready()
> hook in the KASAN core, but it's tricky to get it set up early enough:
> we need it setup before the first call to instrumented code like printk().
> Perhaps in the future.
>
> Only KASAN_MINIMAL works.
>
> Lightly tested on e6500. KVM, kexec and xmon have not been tested.

Hi Daniel,

This is great!

Not related to the patch, but if you booted a real devices and used it
to some degree, I wonder if you hit any KASAN reports?

Thanks

> The test_kasan module fires warnings as expected, except for the
> following tests:
>
>  - Expected/by design:
> kasan test: memcg_accounted_kmem_cache allocate memcg accounted object
>
>  - Due to only supporting KASAN_MINIMAL:
> kasan test: kasan_stack_oob out-of-bounds on stack
> kasan test: kasan_global_oob out-of-bounds global variable
> kasan test: kasan_alloca_oob_left out-of-bounds to left on alloca
> kasan test: kasan_alloca_oob_right out-of-bounds to right on alloca
> kasan test: use_after_scope_test use-after-scope on int
> kasan test: use_after_scope_test use-after-scope on array
>
> Thanks to those who have done the heavy lifting over the past several years:
>  - Christophe's 32 bit series: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-February/185379.html
>  - Aneesh's Book3S hash series: https://lwn.net/Articles/655642/
>  - Balbir's Book3S radix series: https://patchwork.ozlabs.org/patch/795211/
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> ---
>
> While useful if you have a book3e device, this is mostly intended
> as a warm-up exercise for reviving Aneesh's series for book3s hash.
> In particular, changes to the kasan core are going to be required
> for hash and radix as well.
> ---
>  arch/powerpc/Kconfig                         |  1 +
>  arch/powerpc/Makefile                        |  2 +
>  arch/powerpc/include/asm/kasan.h             | 77 ++++++++++++++++++--
>  arch/powerpc/include/asm/ppc_asm.h           |  7 ++
>  arch/powerpc/include/asm/string.h            |  7 +-
>  arch/powerpc/lib/mem_64.S                    |  6 +-
>  arch/powerpc/lib/memcmp_64.S                 |  5 +-
>  arch/powerpc/lib/memcpy_64.S                 |  3 +-
>  arch/powerpc/lib/string.S                    | 15 ++--
>  arch/powerpc/mm/Makefile                     |  2 +
>  arch/powerpc/mm/kasan/Makefile               |  1 +
>  arch/powerpc/mm/kasan/kasan_init_book3e_64.c | 53 ++++++++++++++
>  arch/powerpc/purgatory/Makefile              |  3 +
>  arch/powerpc/xmon/Makefile                   |  1 +
>  14 files changed, 164 insertions(+), 19 deletions(-)
>  create mode 100644 arch/powerpc/mm/kasan/kasan_init_book3e_64.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 850b06def84f..2c7c20d52778 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -176,6 +176,7 @@ config PPC
>         select HAVE_ARCH_AUDITSYSCALL
>         select HAVE_ARCH_JUMP_LABEL
>         select HAVE_ARCH_KASAN                  if PPC32
> +       select HAVE_ARCH_KASAN                  if PPC_BOOK3E_64 && !SPARSEMEM_VMEMMAP
>         select HAVE_ARCH_KGDB
>         select HAVE_ARCH_MMAP_RND_BITS
>         select HAVE_ARCH_MMAP_RND_COMPAT_BITS   if COMPAT
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index f0738099e31e..21c2dadf0315 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -428,11 +428,13 @@ endif
>  endif
>
>  ifdef CONFIG_KASAN
> +ifdef CONFIG_PPC32
>  prepare: kasan_prepare
>
>  kasan_prepare: prepare0
>         $(eval KASAN_SHADOW_OFFSET = $(shell awk '{if ($$2 == "KASAN_SHADOW_OFFSET") print $$3;}' include/generated/asm-offsets.h))
>  endif
> +endif
>
>  # Check toolchain versions:
>  # - gcc-4.6 is the minimum kernel-wide version so nothing required.
> diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
> index 5d0088429b62..c2f6f05dfaa3 100644
> --- a/arch/powerpc/include/asm/kasan.h
> +++ b/arch/powerpc/include/asm/kasan.h
> @@ -5,20 +5,85 @@
>  #ifndef __ASSEMBLY__
>
>  #include <asm/page.h>
> +#include <asm/pgtable.h>
>  #include <asm/pgtable-types.h>
> -#include <asm/fixmap.h>
>
>  #define KASAN_SHADOW_SCALE_SHIFT       3
> -#define KASAN_SHADOW_SIZE      ((~0UL - PAGE_OFFSET + 1) >> KASAN_SHADOW_SCALE_SHIFT)
>
> -#define KASAN_SHADOW_START     (ALIGN_DOWN(FIXADDR_START - KASAN_SHADOW_SIZE, \
> -                                           PGDIR_SIZE))
> -#define KASAN_SHADOW_END       (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
>  #define KASAN_SHADOW_OFFSET    (KASAN_SHADOW_START - \
>                                  (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
> +#define KASAN_SHADOW_END       (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
> +
> +
> +#ifdef CONFIG_PPC32
> +#include <asm/fixmap.h>
> +#define KASAN_SHADOW_START     (ALIGN_DOWN(FIXADDR_START - KASAN_SHADOW_SIZE, \
> +                                           PGDIR_SIZE))
> +#define KASAN_SHADOW_SIZE      ((~0UL - PAGE_OFFSET + 1) >> KASAN_SHADOW_SCALE_SHIFT)
>
>  void kasan_early_init(void);
> +
> +#endif /* CONFIG_PPC32 */
> +
> +#ifdef CONFIG_PPC_BOOK3E_64
> +#define KASAN_SHADOW_START VMEMMAP_BASE
> +#define KASAN_SHADOW_SIZE      (KERN_VIRT_SIZE >> KASAN_SHADOW_SCALE_SHIFT)
> +
> +extern struct static_key_false powerpc_kasan_enabled_key;
> +#define check_return_arch_not_ready() \
> +       do {                                                            \
> +               if (!static_branch_likely(&powerpc_kasan_enabled_key))  \
> +                       return;                                         \
> +       } while (0)
> +
> +extern unsigned char kasan_zero_page[PAGE_SIZE];
> +static inline void *kasan_mem_to_shadow_book3e(const void *addr)
> +{
> +       if ((unsigned long)addr >= KERN_VIRT_START &&
> +               (unsigned long)addr < (KERN_VIRT_START + KERN_VIRT_SIZE)) {
> +               return (void *)kasan_zero_page;
> +       }
> +
> +       return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
> +               + KASAN_SHADOW_OFFSET;
> +}
> +#define kasan_mem_to_shadow kasan_mem_to_shadow_book3e
> +
> +static inline void *kasan_shadow_to_mem_book3e(const void *shadow_addr)
> +{
> +       /*
> +        * We map the entire non-linear virtual mapping onto the zero page so if
> +        * we are asked to map the zero page back just pick the beginning of that
> +        * area.
> +        */
> +       if (shadow_addr >= (void *)kasan_zero_page &&
> +               shadow_addr < (void *)(kasan_zero_page + PAGE_SIZE)) {
> +               return (void *)KERN_VIRT_START;
> +       }
> +
> +       return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET)
> +               << KASAN_SHADOW_SCALE_SHIFT);
> +}
> +#define kasan_shadow_to_mem kasan_shadow_to_mem_book3e
> +
> +static inline bool kasan_addr_has_shadow_book3e(const void *addr)
> +{
> +       /*
> +        * We want to specifically assert that the addresses in the 0x8000...
> +        * region have a shadow, otherwise they are considered by the kasan
> +        * core to be wild pointers
> +        */
> +       if ((unsigned long)addr >= KERN_VIRT_START &&
> +               (unsigned long)addr < (KERN_VIRT_START + KERN_VIRT_SIZE)) {
> +               return true;
> +       }
> +       return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> +}
> +#define kasan_addr_has_shadow kasan_addr_has_shadow_book3e
> +
> +#endif /* CONFIG_PPC_BOOK3E_64 */
> +
>  void kasan_init(void);
>
> -#endif
> +#endif /* CONFIG_KASAN */
>  #endif
> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
> index dba2c1038363..fd7c9fa9d307 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -251,10 +251,17 @@ GLUE(.,name):
>
>  #define _GLOBAL_TOC(name) _GLOBAL(name)
>
> +#endif /* 32-bit */
> +
> +/* KASAN helpers */
>  #define KASAN_OVERRIDE(x, y) \
>         .weak x;             \
>         .set x, y
>
> +#ifdef CONFIG_KASAN
> +#define EXPORT_SYMBOL_NOKASAN(x)
> +#else
> +#define EXPORT_SYMBOL_NOKASAN(x) EXPORT_SYMBOL(x)
>  #endif
>
>  /*
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 64d44d4836b4..e2801d517d57 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -4,13 +4,16 @@
>
>  #ifdef __KERNEL__
>
> +#ifndef CONFIG_KASAN
>  #define __HAVE_ARCH_STRNCPY
>  #define __HAVE_ARCH_STRNCMP
> +#define __HAVE_ARCH_MEMCHR
> +#define __HAVE_ARCH_MEMCMP
> +#endif
> +
>  #define __HAVE_ARCH_MEMSET
>  #define __HAVE_ARCH_MEMCPY
>  #define __HAVE_ARCH_MEMMOVE
> -#define __HAVE_ARCH_MEMCMP
> -#define __HAVE_ARCH_MEMCHR
>  #define __HAVE_ARCH_MEMSET16
>  #define __HAVE_ARCH_MEMCPY_FLUSHCACHE
>
> diff --git a/arch/powerpc/lib/mem_64.S b/arch/powerpc/lib/mem_64.S
> index 3c3be02f33b7..3ff4c6b45505 100644
> --- a/arch/powerpc/lib/mem_64.S
> +++ b/arch/powerpc/lib/mem_64.S
> @@ -30,7 +30,8 @@ EXPORT_SYMBOL(__memset16)
>  EXPORT_SYMBOL(__memset32)
>  EXPORT_SYMBOL(__memset64)
>
> -_GLOBAL(memset)
> +_GLOBAL(__memset)
> +KASAN_OVERRIDE(memset, __memset)
>         neg     r0,r3
>         rlwimi  r4,r4,8,16,23
>         andi.   r0,r0,7                 /* # bytes to be 8-byte aligned */
> @@ -97,7 +98,8 @@ _GLOBAL(memset)
>         blr
>  EXPORT_SYMBOL(memset)
>
> -_GLOBAL_TOC(memmove)
> +_GLOBAL_TOC(__memmove)
> +KASAN_OVERRIDE(memmove, __memmove)
>         cmplw   0,r3,r4
>         bgt     backwards_memcpy
>         b       memcpy
> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
> index 844d8e774492..21aee60de2cd 100644
> --- a/arch/powerpc/lib/memcmp_64.S
> +++ b/arch/powerpc/lib/memcmp_64.S
> @@ -102,7 +102,8 @@
>   * 2) src/dst has different offset to the 8 bytes boundary. The handlers
>   * are named like .Ldiffoffset_xxxx
>   */
> -_GLOBAL_TOC(memcmp)
> +_GLOBAL_TOC(__memcmp)
> +KASAN_OVERRIDE(memcmp, __memcmp)
>         cmpdi   cr1,r5,0
>
>         /* Use the short loop if the src/dst addresses are not
> @@ -630,4 +631,4 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
>         b       .Lcmp_lt32bytes
>
>  #endif
> -EXPORT_SYMBOL(memcmp)
> +EXPORT_SYMBOL_NOKASAN(memcmp)
> diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S
> index 273ea67e60a1..e9092a0e531a 100644
> --- a/arch/powerpc/lib/memcpy_64.S
> +++ b/arch/powerpc/lib/memcpy_64.S
> @@ -18,7 +18,8 @@
>  #endif
>
>         .align  7
> -_GLOBAL_TOC(memcpy)
> +_GLOBAL_TOC(__memcpy)
> +KASAN_OVERRIDE(memcpy, __memcpy)
>  BEGIN_FTR_SECTION
>  #ifdef __LITTLE_ENDIAN__
>         cmpdi   cr7,r5,0
> diff --git a/arch/powerpc/lib/string.S b/arch/powerpc/lib/string.S
> index 4b41970e9ed8..09deaac6e5f1 100644
> --- a/arch/powerpc/lib/string.S
> +++ b/arch/powerpc/lib/string.S
> @@ -16,7 +16,8 @@
>
>  /* This clears out any unused part of the destination buffer,
>     just as the libc version does.  -- paulus */
> -_GLOBAL(strncpy)
> +_GLOBAL(__strncpy)
> +KASAN_OVERRIDE(strncpy, __strncpy)
>         PPC_LCMPI 0,r5,0
>         beqlr
>         mtctr   r5
> @@ -34,9 +35,10 @@ _GLOBAL(strncpy)
>  2:     stbu    r0,1(r6)        /* clear it out if so */
>         bdnz    2b
>         blr
> -EXPORT_SYMBOL(strncpy)
> +EXPORT_SYMBOL_NOKASAN(strncpy)
>
> -_GLOBAL(strncmp)
> +_GLOBAL(__strncmp)
> +KASAN_OVERRIDE(strncmp, __strncmp)
>         PPC_LCMPI 0,r5,0
>         beq-    2f
>         mtctr   r5
> @@ -52,9 +54,10 @@ _GLOBAL(strncmp)
>         blr
>  2:     li      r3,0
>         blr
> -EXPORT_SYMBOL(strncmp)
> +EXPORT_SYMBOL_NOKASAN(strncmp)
>
> -_GLOBAL(memchr)
> +_GLOBAL(__memchr)
> +KASAN_OVERRIDE(memchr, __memchr)
>         PPC_LCMPI 0,r5,0
>         beq-    2f
>         mtctr   r5
> @@ -66,4 +69,4 @@ _GLOBAL(memchr)
>         beqlr
>  2:     li      r3,0
>         blr
> -EXPORT_SYMBOL(memchr)
> +EXPORT_SYMBOL_NOKASAN(memchr)
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 457c0ea2b5e7..d974f7bcb177 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
>  CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>
> +KASAN_SANITIZE_fsl_booke_mmu.o := n
> +
>  obj-y                          := fault.o mem.o pgtable.o mmap.o \
>                                    init_$(BITS).o pgtable_$(BITS).o \
>                                    init-common.o mmu_context.o drmem.o
> diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
> index 6577897673dd..f8f164ad8ade 100644
> --- a/arch/powerpc/mm/kasan/Makefile
> +++ b/arch/powerpc/mm/kasan/Makefile
> @@ -3,3 +3,4 @@
>  KASAN_SANITIZE := n
>
>  obj-$(CONFIG_PPC32)           += kasan_init_32.o
> +obj-$(CONFIG_PPC_BOOK3E_64)   += kasan_init_book3e_64.o
> diff --git a/arch/powerpc/mm/kasan/kasan_init_book3e_64.c b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> new file mode 100644
> index 000000000000..93b9afcf1020
> --- /dev/null
> +++ b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define DISABLE_BRANCH_PROFILING
> +
> +#include <linux/kasan.h>
> +#include <linux/printk.h>
> +#include <linux/memblock.h>
> +#include <linux/sched/task.h>
> +#include <asm/pgalloc.h>
> +
> +DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
> +EXPORT_SYMBOL(powerpc_kasan_enabled_key);
> +unsigned char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
> +
> +static void __init kasan_init_region(struct memblock_region *reg)
> +{
> +       void *start = __va(reg->base);
> +       void *end = __va(reg->base + reg->size);
> +       unsigned long k_start, k_end, k_cur;
> +
> +       if (start >= end)
> +               return;
> +
> +       k_start = (unsigned long)kasan_mem_to_shadow(start);
> +       k_end = (unsigned long)kasan_mem_to_shadow(end);
> +
> +       for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
> +               void *va = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> +               map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
> +       }
> +       flush_tlb_kernel_range(k_start, k_end);
> +}
> +
> +void __init kasan_init(void)
> +{
> +       struct memblock_region *reg;
> +
> +       for_each_memblock(memory, reg)
> +               kasan_init_region(reg);
> +
> +       /* map the zero page RO */
> +       map_kernel_page((unsigned long)kasan_zero_page,
> +                                       __pa(kasan_zero_page), PAGE_KERNEL_RO);
> +
> +       kasan_init_tags();
> +
> +       /* Turn on checking */
> +       static_branch_inc(&powerpc_kasan_enabled_key);
> +
> +       /* Enable error messages */
> +       init_task.kasan_depth = 0;
> +       pr_info("KASAN init done (64-bit Book3E)\n");
> +}
> diff --git a/arch/powerpc/purgatory/Makefile b/arch/powerpc/purgatory/Makefile
> index 4314ba5baf43..7c6d8b14f440 100644
> --- a/arch/powerpc/purgatory/Makefile
> +++ b/arch/powerpc/purgatory/Makefile
> @@ -1,4 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
> +
> +KASAN_SANITIZE := n
> +
>  targets += trampoline.o purgatory.ro kexec-purgatory.c
>
>  LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index 878f9c1d3615..064f7062c0a3 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -6,6 +6,7 @@ subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
>
>  GCOV_PROFILE := n
>  UBSAN_SANITIZE := n
> +KASAN_SANITIZE := n
>
>  # Disable ftrace for the entire directory
>  ORIG_CFLAGS := $(KBUILD_CFLAGS)
> --
> 2.19.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190215000441.14323-6-dja%40axtens.net.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
From: Christophe Leroy @ 2019-02-15  8:41 UTC (permalink / raw)
  To: Daniel Axtens, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <8736oq3u2r.fsf@dja-thinkpad.axtens.net>



Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
> Hi Christophe,
> 
>> --- a/arch/powerpc/include/asm/string.h
>> +++ b/arch/powerpc/include/asm/string.h
>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>>   extern void * memchr(const void *,int,__kernel_size_t);
>>   extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>   
>> +void *__memset(void *s, int c, __kernel_size_t count);
>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>> +
>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>> +/*
>> + * For files that are not instrumented (e.g. mm/slub.c) we
>> + * should use not instrumented version of mem* functions.
>> + */
>> +#define memcpy(dst, src, len) __memcpy(dst, src, len)
>> +#define memmove(dst, src, len) __memmove(dst, src, len)
>> +#define memset(s, c, n) __memset(s, c, n)
>> +#endif
>> +
> 
> I'm finding that I miss tests like 'kasan test: kasan_memcmp
> out-of-bounds in memcmp' because the uninstrumented asm version is used
> instead of an instrumented C version. I ended up guarding the relevant
> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
> the arch versions if we're not compiled with KASAN.
> 
> I find I need to guard and unexport strncpy, strncmp, memchr and
> memcmp. Do you need to do this on 32bit as well, or are those tests
> passing anyway for some reason?

Indeed, I didn't try the KASAN test module recently, because my configs 
don't have CONFIG_MODULE by default.

Trying to test it now, I am discovering that module loading oopses with 
latest version of my series, I need to figure out exactly why. Here 
below the oops by modprobing test_module (the one supposed to just say 
hello to the world).

What we see is an access to the RO kasan zero area.

The shadow mem is 0xf7c00000..0xffc00000
Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.

Why is kasan trying to access that ? Isn't kasan supposed to not check 
stuff in vmalloc area ?

[  189.087499] BUG: Unable to handle kernel data access at 0xf8eb7818
[  189.093455] Faulting instruction address: 0xc001ab60
[  189.098383] Oops: Kernel access of bad area, sig: 11 [#1]
[  189.103732] BE PAGE_SIZE=16K PREEMPT CMPC885
[  189.111414] Modules linked in: test_module(+)
[  189.115817] CPU: 0 PID: 514 Comm: modprobe Not tainted 
5.0.0-rc5-s3k-dev-00645-g1dd3acf23952 #1016
[  189.124627] NIP:  c001ab60 LR: c0194fe8 CTR: 00000003
[  189.129641] REGS: c5645b90 TRAP: 0300   Not tainted 
(5.0.0-rc5-s3k-dev-00645-g1dd3acf23952)
[  189.137924] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 44002884  XER: 00000000
[  189.144571] DAR: f8eb7818 DSISR: 8a000000
[  189.144571] GPR00: c0196620 c5645c40 c5aac000 f8eb7818 00000000 
00000003 f8eb7817 c01950d0
[  189.144571] GPR08: c00c2720 c95bc010 00000000 c95bc1a0 c01965ec 
100d7b30 c0802b80 c5ae0308
[  189.144571] GPR16: c5990480 00000124 0000000f c00bcf7c c5ae0324 
c95bc32c 000006b8 00000001
[  189.144571] GPR24: c95bc364 c95bc360 c95bc2c0 c95bc1a0 00000002 
00000000 00000018 f8eb781b
[  189.182611] NIP [c001ab60] __memset+0xb4/0xc0
[  189.186922] LR [c0194fe8] kasan_unpoison_shadow+0x34/0x54
[  189.192136] Call Trace:
[  189.194682] [c5645c50] [c0196620] __asan_register_globals+0x34/0x74
[  189.200900] [c5645c70] [c00c27a4] do_init_module+0xbc/0x5a4
[  189.206392] [c5645ca0] [c00c0d08] load_module+0x2b5c/0x3194
[  189.211901] [c5645e70] [c00c14c8] sys_init_module+0x188/0x1bc
[  189.217572] [c5645f40] [c001311c] ret_from_syscall+0x0/0x38
[  189.223049] --- interrupt: c01 at 0xfda2b50
[  189.223049]     LR = 0x10014b18
[  189.230175] Instruction dump:
[  189.233117] 4200fffc 70a50003 4d820020 7ca903a6 38c60003 9c860001 
4200fffc 4e800020
[  189.240859] 2c050000 4d820020 7ca903a6 38c3ffff <9c860001> 4200fffc 
4e800020 7c032040
[  189.248809] ---[ end trace 45cbb1b3215e5959 ]---

Christophe

> 
> Regards,
> Daniel
> 
> 
>>   #ifdef CONFIG_PPC64
>>   #define __HAVE_ARCH_MEMSET32
>>   #define __HAVE_ARCH_MEMSET64
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index 879b36602748..fc4c42262694 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
>>   CFLAGS_btext.o		+= -fPIC
>>   endif
>>   
>> -CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> -CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> +CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>>   CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>>   CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>>   
>> @@ -31,6 +32,10 @@ CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
>>   CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
>>   endif
>>   
>> +KASAN_SANITIZE_early_32.o := n
>> +KASAN_SANITIZE_cputable.o := n
>> +KASAN_SANITIZE_prom_init.o := n
>> +
>>   obj-y				:= cputable.o ptrace.o syscalls.o \
>>   				   irq.o align.o signal_32.o pmc.o vdso.o \
>>   				   process.o systbl.o idle.o \
>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>> index 9ffc72ded73a..846fb30b1190 100644
>> --- a/arch/powerpc/kernel/asm-offsets.c
>> +++ b/arch/powerpc/kernel/asm-offsets.c
>> @@ -783,5 +783,9 @@ int main(void)
>>   	DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>>   #endif
>>   
>> +#ifdef CONFIG_KASAN
>> +	DEFINE(KASAN_SHADOW_OFFSET, KASAN_SHADOW_OFFSET);
>> +#endif
>> +
>>   	return 0;
>>   }
>> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
>> index 05b08db3901d..0ec9dec06bc2 100644
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -962,6 +962,9 @@ start_here:
>>    * Do early platform-specific initialization,
>>    * and set up the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
>> index b19d78410511..5d6ff8fa7e2b 100644
>> --- a/arch/powerpc/kernel/head_40x.S
>> +++ b/arch/powerpc/kernel/head_40x.S
>> @@ -848,6 +848,9 @@ start_here:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
>> index bf23c19c92d6..7ca14dff6192 100644
>> --- a/arch/powerpc/kernel/head_44x.S
>> +++ b/arch/powerpc/kernel/head_44x.S
>> @@ -203,6 +203,9 @@ _ENTRY(_start);
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
>> index 0fea10491f3a..6a644ea2e6b6 100644
>> --- a/arch/powerpc/kernel/head_8xx.S
>> +++ b/arch/powerpc/kernel/head_8xx.S
>> @@ -823,6 +823,9 @@ start_here:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
>> index 2386ce2a9c6e..4f4585a68850 100644
>> --- a/arch/powerpc/kernel/head_fsl_booke.S
>> +++ b/arch/powerpc/kernel/head_fsl_booke.S
>> @@ -274,6 +274,9 @@ set_ivor:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	mr	r3,r30
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
>> index 667df97d2595..da6bb16e0876 100644
>> --- a/arch/powerpc/kernel/prom_init_check.sh
>> +++ b/arch/powerpc/kernel/prom_init_check.sh
>> @@ -16,8 +16,16 @@
>>   # If you really need to reference something from prom_init.o add
>>   # it to the list below:
>>   
>> +grep CONFIG_KASAN=y .config >/dev/null
>> +if [ $? -eq 0 ]
>> +then
>> +	MEMFCT="__memcpy __memset"
>> +else
>> +	MEMFCT="memcpy memset"
>> +fi
>> +
>>   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>> -_end enter_prom memcpy memset reloc_offset __secondary_hold
>> +_end enter_prom $MEMFCT reloc_offset __secondary_hold
>>   __secondary_hold_acknowledge __secondary_hold_spinloop __start
>>   strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
>>   reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index ca00fbb97cf8..16ff1ea66805 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -978,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
>>   
>>   	paging_init();
>>   
>> +	kasan_init();
>> +
>>   	/* Initialize the MMU context management stuff. */
>>   	mmu_context_init();
>>   
>> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> index 3bf9fc6fd36c..ce8d4a9f810a 100644
>> --- a/arch/powerpc/lib/Makefile
>> +++ b/arch/powerpc/lib/Makefile
>> @@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
>>   CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>>   
>> +KASAN_SANITIZE_code-patching.o := n
>> +KASAN_SANITIZE_feature-fixups.o := n
>> +
>> +ifdef CONFIG_KASAN
>> +CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
>> +endif
>> +
>>   obj-y += string.o alloc.o code-patching.o feature-fixups.o
>>   
>>   obj-$(CONFIG_PPC32)	+= div64.o copy_32.o crtsavres.o strlen_32.o
>> diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
>> index ba66846fe973..4d8a1c73b4cf 100644
>> --- a/arch/powerpc/lib/copy_32.S
>> +++ b/arch/powerpc/lib/copy_32.S
>> @@ -91,7 +91,8 @@ EXPORT_SYMBOL(memset16)
>>    * We therefore skip the optimised bloc that uses dcbz. This jump is
>>    * replaced by a nop once cache is active. This is done in machine_init()
>>    */
>> -_GLOBAL(memset)
>> +_GLOBAL(__memset)
>> +KASAN_OVERRIDE(memset, __memset)
>>   	cmplwi	0,r5,4
>>   	blt	7f
>>   
>> @@ -163,12 +164,14 @@ EXPORT_SYMBOL(memset)
>>    * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
>>    * replaced by a nop once cache is active. This is done in machine_init()
>>    */
>> -_GLOBAL(memmove)
>> +_GLOBAL(__memmove)
>> +KASAN_OVERRIDE(memmove, __memmove)
>>   	cmplw	0,r3,r4
>>   	bgt	backwards_memcpy
>>   	/* fall through */
>>   
>> -_GLOBAL(memcpy)
>> +_GLOBAL(__memcpy)
>> +KASAN_OVERRIDE(memcpy, __memcpy)
>>   1:	b	generic_memcpy
>>   	patch_site	1b, patch__memcpy_nocache
>>   
>> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
>> index f965fc33a8b7..d6b76f25f6de 100644
>> --- a/arch/powerpc/mm/Makefile
>> +++ b/arch/powerpc/mm/Makefile
>> @@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   
>>   CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>>   
>> +KASAN_SANITIZE_kasan_init.o := n
>> +
>>   obj-y				:= fault.o mem.o pgtable.o mmap.o \
>>   				   init_$(BITS).o pgtable_$(BITS).o \
>>   				   init-common.o mmu_context.o drmem.o
>> @@ -55,3 +57,4 @@ obj-$(CONFIG_PPC_BOOK3S_64)	+= dump_linuxpagetables-book3s64.o
>>   endif
>>   obj-$(CONFIG_PPC_HTDUMP)	+= dump_hashpagetable.o
>>   obj-$(CONFIG_PPC_MEM_KEYS)	+= pkeys.o
>> +obj-$(CONFIG_KASAN)		+= kasan_init.o
>> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
>> index 6aa41669ac1a..c862b48118f1 100644
>> --- a/arch/powerpc/mm/dump_linuxpagetables.c
>> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
>> @@ -94,6 +94,10 @@ static struct addr_marker address_markers[] = {
>>   	{ 0,	"Consistent mem start" },
>>   	{ 0,	"Consistent mem end" },
>>   #endif
>> +#ifdef CONFIG_KASAN
>> +	{ 0,	"kasan shadow mem start" },
>> +	{ 0,	"kasan shadow mem end" },
>> +#endif
>>   #ifdef CONFIG_HIGHMEM
>>   	{ 0,	"Highmem PTEs start" },
>>   	{ 0,	"Highmem PTEs end" },
>> @@ -310,6 +314,10 @@ static void populate_markers(void)
>>   	address_markers[i++].start_address = IOREMAP_TOP +
>>   					     CONFIG_CONSISTENT_SIZE;
>>   #endif
>> +#ifdef CONFIG_KASAN
>> +	address_markers[i++].start_address = KASAN_SHADOW_START;
>> +	address_markers[i++].start_address = KASAN_SHADOW_END;
>> +#endif
>>   #ifdef CONFIG_HIGHMEM
>>   	address_markers[i++].start_address = PKMAP_BASE;
>>   	address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
>> diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
>> new file mode 100644
>> index 000000000000..bd8e0a263e12
>> --- /dev/null
>> +++ b/arch/powerpc/mm/kasan_init.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define DISABLE_BRANCH_PROFILING
>> +
>> +#include <linux/kasan.h>
>> +#include <linux/printk.h>
>> +#include <linux/memblock.h>
>> +#include <linux/sched/task.h>
>> +#include <asm/pgalloc.h>
>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	unsigned long addr = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	unsigned long next;
>> +	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
>> +	int i;
>> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
>> +
>> +	BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
>> +
>> +	if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>> +		panic("KASAN not supported with Hash MMU\n");
>> +
>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		__set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>> +			     kasan_early_shadow_pte + i,
>> +			     pfn_pte(PHYS_PFN(pa), PAGE_KERNEL), 0);
>> +
>> +	do {
>> +		next = pgd_addr_end(addr, end);
>> +		pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
>> +	} while (pmd++, addr = next, addr != end);
>> +}
>> +
>> +static void __init kasan_init_region(struct memblock_region *reg)
>> +{
>> +	void *start = __va(reg->base);
>> +	void *end = __va(reg->base + reg->size);
>> +	unsigned long k_start, k_end, k_cur, k_next;
>> +	pmd_t *pmd;
>> +	void *block;
>> +
>> +	if (start >= end)
>> +		return;
>> +
>> +	k_start = (unsigned long)kasan_mem_to_shadow(start);
>> +	k_end = (unsigned long)kasan_mem_to_shadow(end);
>> +	pmd = pmd_offset(pud_offset(pgd_offset_k(k_start), k_start), k_start);
>> +
>> +	for (k_cur = k_start; k_cur != k_end; k_cur = k_next, pmd++) {
>> +		k_next = pgd_addr_end(k_cur, k_end);
>> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte) {
>> +			pte_t *new = pte_alloc_one_kernel(&init_mm);
>> +
>> +			if (!new)
>> +				panic("kasan: pte_alloc_one_kernel() failed");
>> +			memcpy(new, kasan_early_shadow_pte, PTE_TABLE_SIZE);
>> +			pmd_populate_kernel(&init_mm, pmd, new);
>> +		}
>> +	};
>> +
>> +	block = memblock_alloc(k_end - k_start, PAGE_SIZE);
>> +	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
>> +		void *va = block ? block + k_cur - k_start :
>> +				   memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>> +		pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
>> +
>> +		if (!va)
>> +			panic("kasan: memblock_alloc() failed");
>> +		pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
>> +		pte_update(pte_offset_kernel(pmd, k_cur), ~0, pte_val(pte));
>> +	}
>> +	flush_tlb_kernel_range(k_start, k_end);
>> +}
>> +
>> +static void __init kasan_remap_early_shadow_ro(void)
>> +{
>> +	unsigned long k_cur;
>> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
>> +	int i;
>> +
>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		ptep_set_wrprotect(&init_mm, 0, kasan_early_shadow_pte + i);
>> +
>> +	for (k_cur = PAGE_OFFSET & PAGE_MASK; k_cur; k_cur += PAGE_SIZE) {
>> +		pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
>> +		pte_t *ptep = pte_offset_kernel(pmd, k_cur);
>> +
>> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte)
>> +			continue;
>> +		if ((pte_val(*ptep) & PAGE_MASK) != pa)
>> +			continue;
>> +
>> +		ptep_set_wrprotect(&init_mm, k_cur, ptep);
>> +	}
>> +	flush_tlb_mm(&init_mm);
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	for_each_memblock(memory, reg)
>> +		kasan_init_region(reg);
>> +
>> +	kasan_remap_early_shadow_ro();
>> +
>> +	clear_page(kasan_early_shadow_page);
>> +
>> +	/* At this point kasan is fully initialized. Enable error messages */
>> +	init_task.kasan_depth = 0;
>> +	pr_info("KASAN init done\n");
>> +}
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index 81f251fc4169..1bb055775e60 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -336,6 +336,10 @@ void __init mem_init(void)
>>   	pr_info("  * 0x%08lx..0x%08lx  : highmem PTEs\n",
>>   		PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
>>   #endif /* CONFIG_HIGHMEM */
>> +#ifdef CONFIG_KASAN
>> +	pr_info("  * 0x%08lx..0x%08lx  : kasan shadow mem\n",
>> +		KASAN_SHADOW_START, KASAN_SHADOW_END);
>> +#endif
>>   #ifdef CONFIG_NOT_COHERENT_CACHE
>>   	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
>>   		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
>> -- 
>> 2.13.3

^ permalink raw reply

* Re: [PATCH 08/11] lib: consolidate the GENERIC_BUG symbol
From: Masahiro Yamada @ 2019-02-15  8:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-9-hch@lst.de>

On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> And just let the architectures that want it select the symbol.
> Same for GENERIC_BUG_RELATIVE_POINTERS.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>


This slightly changes the behavior of GENERIC_BUG_RELATIVE_POINTERS
for arm64, riscv, x86.
Previously, GENERIC_BUG_RELATIVE_POINTERS was enabled only when BUG=y.


Having said that, this is not a big deal.
When CONFIG_GENERIC_BUG=n, CONFIG_GENERIC_BUG_RELATIVE_POINTERS
is actually don't care.



If you change this,
could you add some comments in commit description?


> ---

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c39dac831f08..913b2ca7ec22 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -85,6 +85,8 @@ config ARM64
>         select FRAME_POINTER
>         select GENERIC_ALLOCATOR
>         select GENERIC_ARCH_TOPOLOGY
> +       select GENERIC_BUG if BUG
> +       select GENERIC_BUG_RELATIVE_POINTERS

Precisely,

          select GENERIC_BUG_RELATIVE_POINTERS if BUG




> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 732614eb3683..c410ed896567 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -19,6 +19,8 @@ config RISCV
>         select ARCH_WANT_FRAME_POINTERS
>         select CLONE_BACKWARDS
>         select COMMON_CLK
> +       select GENERIC_BUG if BUG
> +       select GENERIC_BUG_RELATIVE_POINTERS if 64BIT

Precisely,

          select GENERIC_BUG_RELATIVE_POINTERS if 64BIT && BUG


> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 15ccdd04814e..2a5c12be633e 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -17,12 +17,6 @@ config ARCH_HAS_ILOG2_U64
>  config GENERIC_HWEIGHT
>         def_bool y
>
> -config GENERIC_BUG
> -       def_bool y if BUG
> -
> -config GENERIC_BUG_RELATIVE_POINTERS
> -       def_bool y
> -


Hmm, s390 enables GENERIC_BUG_RELATIVE_POINTERS
irrespective of BUG...





> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 1bd4f19b6b28..f4cb31174e1b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -91,6 +91,8 @@ config X86
>         select DCACHE_WORD_ACCESS
>         select EDAC_ATOMIC_SCRUB
>         select EDAC_SUPPORT
> +       select GENERIC_BUG                      if BUG
> +       select GENERIC_BUG_RELATIVE_POINTERS    if X86_64

Precisely,

          select GENERIC_BUG_RELATIVE_POINTERS    if X86_64 && BUG






--
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH] powerpc/ptrace: Add prototype for function pt_regs_check
From: Mathieu Malaterre @ 2019-02-15  8:46 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev, Oleg Nesterov, Paul Mackerras, LKML
In-Reply-To: <e95cb3ea-019f-28b0-0af0-ad8c80b3106b@c-s.fr>

On Fri, Feb 15, 2019 at 9:21 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 15/02/2019 à 09:11, Mathieu Malaterre a écrit :
> > On Sat, Dec 8, 2018 at 4:46 PM Mathieu Malaterre <malat@debian.org> wrote:
> >>
> >> `pt_regs_check` is a dummy function, its purpose is to break the build
> >> if struct pt_regs and struct user_pt_regs don't match.
> >>
> >> This function has no functionnal purpose, and will get eliminated at
> >> link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> >>
> >> This commit adds a prototype to fix warning at W=1:
> >>
> >>    arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for ‘pt_regs_check’ [-Werror=missing-prototypes]
> >>
> >> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> >> ---
> >>   arch/powerpc/kernel/ptrace.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> >> index a398999d0770..341c0060b4c8 100644
> >> --- a/arch/powerpc/kernel/ptrace.c
> >> +++ b/arch/powerpc/kernel/ptrace.c
> >> @@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
> >>          user_enter();
> >>   }
> >>
> >> +void __init pt_regs_check(void);
> >> +/* dummy function, its purpose is to break the build if struct pt_regs and
> >> + * struct user_pt_regs don't match.
> >> + */
> >
> > Another trick which seems to work with GCC is:
> >
> > -void __init pt_regs_check(void)
> > +static inline void __init pt_regs_check(void)
>
> Does this really work ? Did you test to ensure that the BUILD_BUG_ON
> still detect mismatch between struct pt_regs and struct user_pt_regs ?
>

My bad, I was unaware of GCC behavior for static inline in this case.
Sorry for the noise.
Original ugly patch does work though.
>
> >
> >>   void __init pt_regs_check(void)
> >>   {
> >>          BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
> >> --
> >> 2.19.2
> >>

^ permalink raw reply

* Re: [PATCH] mmap.2: describe the 5level paging hack
From: Michael Ellerman @ 2019-02-15  9:13 UTC (permalink / raw)
  To: Jann Horn, mtk.manpages, jannh
  Cc: linux-arch, linux-man, linux-mm, Peter Zijlstra, Aneesh Kumar K.V,
	Will Deacon, linuxppc-dev, Andy Lutomirski, Dave Hansen,
	Paul Mackerras, linux-arm-kernel, Catalin Marinas, Andrew Morton,
	linux-api, Linus Torvalds, Thomas Gleixner, Kirill A . Shutemov
In-Reply-To: <20190211163653.97742-1-jannh@google.com>

Jann Horn <jannh@google.com> writes:

> The manpage is missing information about the compatibility hack for
> 5-level paging that went in in 4.14, around commit ee00f4a32a76 ("x86/mm:
> Allow userspace have mappings above 47-bit"). Add some information about
> that.

Thanks for doing this.

> While I don't think any hardware supporting this is shipping yet (?), I
> think it's useful to try to write a manpage for this API, partly to
> figure out how usable that API actually is, and partly because when this
> hardware does ship, it'd be nice if distro manpages had information about
> how to use it.
>
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> This patch goes on top of the patch "[PATCH] mmap.2: fix description of
> treatment of the hint" that I just sent, but I'm not sending them in a
> series because I want the first one to go in, and I think this one might
> be a bit more controversial.
>
> It would be nice if the architecture maintainers and mm folks could have
> a look at this and check that what I wrote is right - I only looked at
> the source for this, I haven't tried it.
>
>  man2/mmap.2 | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 8556bbfeb..977782fa8 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -67,6 +67,8 @@ is NULL,
>  then the kernel chooses the (page-aligned) address
>  at which to create the mapping;
>  this is the most portable method of creating a new mapping.
> +On Linux, in this case, the kernel may limit the maximum address that can be
> +used for allocations to a legacy limit for compatibility reasons.
>  If
>  .I addr
>  is not NULL,
> @@ -77,6 +79,19 @@ or equal to the value specified by
>  and attempt to create the mapping there.
>  If another mapping already exists there, the kernel picks a new
>  address, independent of the hint.
> +However, if a hint above the architecture's legacy address limit is provided
> +(on x86-64: above 0x7ffffffff000, on arm64: above 0x1000000000000, on ppc64 with
> +book3s: above 0x7fffffffffff or 0x3fffffffffff, depending on page size), the

It doesn't depend on page size for ppc64(le). With 4K pages the user VM
is always 64TB.

So the only boundary for us is at 128T when using 64K pages.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/ptrace: Simplify vr_get/set() to avoid GCC warning
From: Michael Ellerman @ 2019-02-15  9:20 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: linuxppc-dev, Meelis Roos
In-Reply-To: <CA+7wUszvErrmyo9jAuPk0dtvwKd3cw4jSb04z5Q4DkGEjGiY7w@mail.gmail.com>

Mathieu Malaterre <malat@debian.org> writes:
> On Fri, Feb 15, 2019 at 7:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> GCC 8 warns about the logic in vr_get/set(), which with -Werror breaks
>> the build:
>>
>>   In function ‘user_regset_copyin’,
>>       inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:628:9:
>>   include/linux/regset.h:295:4: error: ‘memcpy’ offset [-527, -529] is
>>   out of the bounds [0, 16] of object ‘vrsave’ with type ‘union
>>   <anonymous>’ [-Werror=array-bounds]
>>   arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
>>   arch/powerpc/kernel/ptrace.c:623:5: note: ‘vrsave’ declared here
>>      } vrsave;
>>
>> This has been identified as a regression in GCC, see GCC bug 88273.
>
> Good point, this does not seems this will be backported.
>
>> However we can avoid the warning and also simplify the logic and make
>> it more robust.
>>
>> Currently we pass -1 as end_pos to user_regset_copyout(). This says
>> "copy up to the end of the regset".
>>
>> The definition of the regset is:
>>         [REGSET_VMX] = {
>>                 .core_note_type = NT_PPC_VMX, .n = 34,
>>                 .size = sizeof(vector128), .align = sizeof(vector128),
>>                 .active = vr_active, .get = vr_get, .set = vr_set
>>         },
>>
>> The end is calculated as (n * size), ie. 34 * sizeof(vector128).
>>
>> In vr_get/set() we pass start_pos as 33 * sizeof(vector128), meaning
>> we can copy up to sizeof(vector128) into/out-of vrsave.
>>
>> The on-stack vrsave is defined as:
>>   union {
>>           elf_vrreg_t reg;
>>           u32 word;
>>   } vrsave;
>>
>> And elf_vrreg_t is:
>>   typedef __vector128 elf_vrreg_t;
>>
>> So there is no bug, but we rely on all those sizes lining up,
>> otherwise we would have a kernel stack exposure/overwrite on our
>> hands.
>>
>> Rather than relying on that we can pass an explict end_pos based on
>> the sizeof(vrsave). The result should be exactly the same but it's
>> more obviously not over-reading/writing the stack and it avoids the
>> compiler warning.
>>
>
> maybe:
>
> Link: https://lkml.org/lkml/2018/8/16/117

Hmm, I prefer not to include links because they're unlikely to last
forever like the git history.

If we do include them the preferred form is a link to lkml.kernel.org
using the message id. That way the message is recorded and also the link
is "guaranteed" to work in future, eg:

http://lkml.kernel.org/r/alpine.LRH.2.21.1808161041350.16413@math.ut.ee

In this case I don't think the link adds much over what I have in the
change log, in particular I did credit Meelis as the reporter.

> In any case the warning is now gone:
>
> Tested-by: Mathieu Malaterre <malat@debian.org>

Thanks.

cheers

>> Reported-by: Meelis Roos <mroos@linux.ee>
>> Reported-by: Mathieu Malaterre <malat@debian.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

^ permalink raw reply

* Re: [PATCH 01/11] powerpc: remove dead ifdefs in <asm/checksum.h>
From: Masahiro Yamada @ 2019-02-15  9:30 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-s390, linux-xtensa, Linux Kbuild mailing list,
	Linux Kernel Mailing List, linux-riscv, linuxppc-dev,
	Christoph Hellwig
In-Reply-To: <d5451fc4-9183-702a-e9ed-4a2be1f82a11@c-s.fr>

On Fri, Feb 15, 2019 at 5:18 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 14/02/2019 à 18:05, Christoph Hellwig a écrit :
> > On Thu, Feb 14, 2019 at 09:26:19AM +0100, Christophe Leroy wrote:
> >> Could you also remove the 'config GENERIC_CSUM' item in
> >> arch/powerpc/Kconfig ?
> >
> > All the separate declarations go away later in this series.
> >
>
> I saw, but the purpose of the later patch is to replace arch defined
> GENERIC_CSUM by a common one that arches select. For the powerpc you are
> not in that case as the powerpc does not select GENERIC_CSUM.
>
> So I really believe that all stale bits of remaining GENERIC_CSUM in
> powerpc should go away as a single dedicated patch, as a fix of commit
> d4fde568a34a ("powerpc/64: Use optimized checksum routines on
> little-endian")
>
> Regarding the #ifdef __KERNEL__ , I think we should do a wide cleanup in
> arch/powerpc/include/asm, not only asm/checksum.h
>
> Christophe


Please send such cleanups to PowerPC ML
instead of to me (Kbuild).


Christoph,
I think this one is independent of the rest of this series.
How about separating it if you volunteer to Powerpc cleansup?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* [PATCH] powerpc: use $(origin ARCH) to select KBUILD_DEFCONFIG
From: Masahiro Yamada @ 2019-02-15  9:38 UTC (permalink / raw)
  To: linuxppc-dev, Michael Ellerman
  Cc: Masahiro Yamada, Paul Mackerras, linux-kernel

I often test all Kconfig commands for all architectures. To ease my
workflow, I want 'make defconfig' at least working without any cross
compiler.

Currently, arch/powerpc/Makefile checks CROSS_COMPILE to decide the
default defconfig source.

If CROSS_COMPILE is unset, it is likely to be the native build, so
'uname -m' is useful to choose the defconfig. If CROSS_COMPILE is set,
the user is cross-building (i.e. 'uname -m' is probably x86_64), so
it falls back to ppc64_defconfig. Yup, make sense.

However, I want to run 'make ARCH=* defconfig' without setting
CROSS_COMPILE for each architecture.

My suggestion is to check $(origin ARCH).

When you cross-compile the kernel, you need to set ARCH from your
environment or from the command line.

For the native build, you do not need to set ARCH. The default in
the top Makefile is used:

  ARCH            ?= $(SUBARCH)

Hence, $(origin ARCH) returns 'file'.

Before this commit, 'make ARCH=powerpc defconfig' failed:

  $ make ARCH=powerpc defconfig
  *** Default configuration is based on target 'x86_64_defconfig'
  ***
  *** Can't find default configuration "arch/powerpc/configs/x86_64_defconfig"!
  ***

After this commit, it will succeed:

  $ make ARCH=powerpc defconfig
  *** Default configuration is based on 'ppc64_defconfig'
  #
  # configuration written to .config
  #

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/powerpc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ac03334..f989979 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -34,7 +34,7 @@ ifdef CONFIG_PPC_BOOK3S_32
 KBUILD_CFLAGS		+= -mcpu=powerpc
 endif
 
-ifeq ($(CROSS_COMPILE),)
+ifeq ($(origin ARCH), file)
 KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
 else
 KBUILD_DEFCONFIG := ppc64_defconfig
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH 02/11] riscv: remove the HAVE_KPROBES option
From: Masahiro Yamada @ 2019-02-15  9:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xtensa, Linux Kbuild mailing list, linux-s390,
	Linux Kernel Mailing List, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-3-hch@lst.de>

On Thu, Feb 14, 2019 at 2:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> HAVE_KPROBES is defined genericly in arch/Kconfig and architectures
> should just select it if supported.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Do you want this patch picked up by me?

Or, by Palmer?



> ---
>  arch/riscv/Kconfig | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 515fc3cc9687..b60f4e3e36f4 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -94,9 +94,6 @@ config PGTABLE_LEVELS
>         default 3 if 64BIT
>         default 2
>
> -config HAVE_KPROBES
> -       def_bool n
> -
>  menu "Platform type"
>
>  choice
> --
> 2.20.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox