public inbox for linux-next@vger.kernel.org 
 help / color / mirror / Atom feed
* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
       [not found] ` <20201019084140.4532-3-linus.walleij@linaro.org>
@ 2020-11-06  7:49   ` Naresh Kamboju
  2020-11-06  8:26     ` Linus Walleij
  0 siblings, 1 reply; 16+ messages in thread
From: Naresh Kamboju @ 2020-11-06  7:49 UTC (permalink / raw)
  To: Linus Walleij, Linux-Next Mailing List
  Cc: Florian Fainelli, Abbott Liu, Russell King, Ard Biesheuvel,
	Andrey Ryabinin, Mike Rapoport, Ahmad Fatoum, Arnd Bergmann,
	kasan-dev, Alexander Potapenko, Linux ARM, Dmitry Vyukov,
	Stephen Rothwell

On Mon, 19 Oct 2020 at 14:14, Linus Walleij <linus.walleij@linaro•org> wrote:
>
> From: Andrey Ryabinin <aryabinin@virtuozzo•com>
>
> Functions like memset()/memmove()/memcpy() do a lot of memory
> accesses.
>
> If a bad pointer is passed to one of these functions it is important
> to catch this. Compiler instrumentation cannot do this since these
> functions are written in assembly.
>
> KASan replaces these memory functions with instrumented variants.
>
> The original functions are declared as weak symbols so that
> the strong definitions in mm/kasan/kasan.c can replace them.
>
> The original functions have aliases with a '__' prefix in their
> name, so we can call the non-instrumented variant if needed.
>
> We must use __memcpy()/__memset() in place of memcpy()/memset()
> when we copy .data to RAM and when we clear .bss, because
> kasan_early_init cannot be called before the initialization of
> .data and .bss.
>
> For the kernel compression and EFI libstub's custom string
> libraries we need a special quirk: even if these are built
> without KASan enabled, they rely on the global headers for their
> custom string libraries, which means that e.g. memcpy()
> will be defined to __memcpy() and we get link failures.
> Since these implementations are written i C rather than
> assembly we use e.g. __alias(memcpy) to redirected any
> users back to the local implementation.
>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo•com>
> Cc: Alexander Potapenko <glider@google•com>
> Cc: Dmitry Vyukov <dvyukov@google•com>
> Cc: kasan-dev@googlegroups•com
> Reviewed-by: Ard Biesheuvel <ardb@kernel•org>
> Tested-by: Ard Biesheuvel <ardb@kernel•org> # QEMU/KVM/mach-virt/LPAE/8G
> Tested-by: Florian Fainelli <f.fainelli@gmail•com> # Brahma SoCs
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix•de> # i.MX6Q
> Reported-by: Russell King - ARM Linux <linux@armlinux•org.uk>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix•de>
> Signed-off-by: Abbott Liu <liuwenliang@huawei•com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail•com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro•org>
> ---
> ChangeLog v15->v16:
> - Fold in Ahmad Fatoum's fixup for fortify
> - Collect Florian's Tested-by
> - Resend with the other patches
> ChangeLog v14->v15:
> - Resend with the other patches
> ChangeLog v13->v14:
> - Resend with the other patches
> ChangeLog v12->v13:
> - Rebase on kernel v5.9-rc1
> ChangeLog v11->v12:
> - Resend with the other changes.
> ChangeLog v10->v11:
> - Resend with the other changes.
> ChangeLog v9->v10:
> - Rebase on v5.8-rc1
> ChangeLog v8->v9:
> - Collect Ard's tags.
> ChangeLog v7->v8:
> - Use the less invasive version of handling the global redefines
>   of the string functions in the decompressor: __alias() the
>   functions locally in the library.
> - Put in some more comments so readers of the code knows what
>   is going on.
> ChangeLog v6->v7:
> - Move the hacks around __SANITIZE_ADDRESS__ into this file
> - Edit the commit message
> - Rebase on the other v2 patches
> ---
>  arch/arm/boot/compressed/string.c | 19 +++++++++++++++++++
>  arch/arm/include/asm/string.h     | 26 ++++++++++++++++++++++++++
>  arch/arm/kernel/head-common.S     |  4 ++--
>  arch/arm/lib/memcpy.S             |  3 +++
>  arch/arm/lib/memmove.S            |  5 ++++-
>  arch/arm/lib/memset.S             |  3 +++
>  6 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> index ade5079bebbf..8c0fa276d994 100644
> --- a/arch/arm/boot/compressed/string.c
> +++ b/arch/arm/boot/compressed/string.c
> @@ -7,6 +7,25 @@
>
>  #include <linux/string.h>
>
> +/*
> + * The decompressor is built without KASan but uses the same redirects as the
> + * rest of the kernel when CONFIG_KASAN is enabled, defining e.g. memcpy()
> + * to __memcpy() but since we are not linking with the main kernel string
> + * library in the decompressor, that will lead to link failures.
> + *
> + * Undefine KASan's versions, define the wrapped functions and alias them to
> + * the right names so that when e.g. __memcpy() appear in the code, it will
> + * still be linked to this local version of memcpy().
> + */
> +#ifdef CONFIG_KASAN
> +#undef memcpy
> +#undef memmove
> +#undef memset
> +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> +void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> +void *__memset(void *s, int c, size_t count) __alias(memset);
> +#endif
> +
>  void *memcpy(void *__dest, __const void *__src, size_t __n)

arm KASAN build failure noticed on linux next 20201106 tag.
gcc: 9.x

Build error:
---------------
arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias'
argument not a string
   24 | void *__memcpy(void *__dest, __const void *__src, size_t __n)
__alias(memcpy);
      | ^~~~
arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias'
argument not a string
   25 | void *__memmove(void *__dest, __const void *__src, size_t
count) __alias(memmove);
      | ^~~~
arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias'
argument not a string
   26 | void *__memset(void *s, int c, size_t count) __alias(memset);
      | ^~~~

Reported-by: Naresh Kamboju <naresh.kamboju@linaro•org>

Build details link,
https://builds.tuxbuild.com/1juBs4tXRA6Cwhd1Qnhh4vzCtDx/

-- 
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06  7:49   ` [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan Naresh Kamboju
@ 2020-11-06  8:26     ` Linus Walleij
  2020-11-06  8:28       ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2020-11-06  8:26 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: Linux-Next Mailing List, Florian Fainelli, Abbott Liu,
	Russell King, Ard Biesheuvel, Andrey Ryabinin, Mike Rapoport,
	Ahmad Fatoum, Arnd Bergmann, kasan-dev, Alexander Potapenko,
	Linux ARM, Dmitry Vyukov, Stephen Rothwell

On Fri, Nov 6, 2020 at 8:49 AM Naresh Kamboju <naresh.kamboju@linaro•org> wrote:

> arm KASAN build failure noticed on linux next 20201106 tag.
> gcc: 9.x
>
> Build error:
> ---------------
> arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias'
> argument not a string
>    24 | void *__memcpy(void *__dest, __const void *__src, size_t __n)
> __alias(memcpy);
>       | ^~~~
> arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias'
> argument not a string
>    25 | void *__memmove(void *__dest, __const void *__src, size_t
> count) __alias(memmove);
>       | ^~~~
> arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias'
> argument not a string
>    26 | void *__memset(void *s, int c, size_t count) __alias(memset);
>       | ^~~~
>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro•org>
>
> Build details link,
> https://builds.tuxbuild.com/1juBs4tXRA6Cwhd1Qnhh4vzCtDx/

This looks like a randconfig build.

Please drill down and try to report which combination of config
options that give rise to this problem so we have a chance of
amending it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06  8:26     ` Linus Walleij
@ 2020-11-06  8:28       ` Ard Biesheuvel
  2020-11-06  9:44         ` Nathan Chancellor
  0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2020-11-06  8:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Naresh Kamboju, Linux-Next Mailing List, Florian Fainelli,
	Abbott Liu, Russell King, Andrey Ryabinin, Mike Rapoport,
	Ahmad Fatoum, Arnd Bergmann, kasan-dev, Alexander Potapenko,
	Linux ARM, Dmitry Vyukov, Stephen Rothwell

On Fri, 6 Nov 2020 at 09:26, Linus Walleij <linus.walleij@linaro•org> wrote:
>
> On Fri, Nov 6, 2020 at 8:49 AM Naresh Kamboju <naresh.kamboju@linaro•org> wrote:
>
> > arm KASAN build failure noticed on linux next 20201106 tag.
> > gcc: 9.x
> >
> > Build error:
> > ---------------
> > arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias'
> > argument not a string
> >    24 | void *__memcpy(void *__dest, __const void *__src, size_t __n)
> > __alias(memcpy);
> >       | ^~~~
> > arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias'
> > argument not a string
> >    25 | void *__memmove(void *__dest, __const void *__src, size_t
> > count) __alias(memmove);
> >       | ^~~~
> > arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias'
> > argument not a string
> >    26 | void *__memset(void *s, int c, size_t count) __alias(memset);
> >       | ^~~~
> >
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro•org>
> >
> > Build details link,
> > https://builds.tuxbuild.com/1juBs4tXRA6Cwhd1Qnhh4vzCtDx/
>
> This looks like a randconfig build.
>
> Please drill down and try to report which combination of config
> options that give rise to this problem so we have a chance of
> amending it.
>

AFAIK there is an incompatible change in -next to change the
definition of the __alias() macro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06  8:28       ` Ard Biesheuvel
@ 2020-11-06  9:44         ` Nathan Chancellor
  2020-11-06 13:37           ` Linus Walleij
  0 siblings, 1 reply; 16+ messages in thread
From: Nathan Chancellor @ 2020-11-06  9:44 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linus Walleij, Naresh Kamboju, Linux-Next Mailing List,
	Florian Fainelli, Abbott Liu, Russell King, Andrey Ryabinin,
	Mike Rapoport, Ahmad Fatoum, Arnd Bergmann, kasan-dev,
	Alexander Potapenko, Linux ARM, Dmitry Vyukov, Stephen Rothwell

On Fri, Nov 06, 2020 at 09:28:09AM +0100, Ard Biesheuvel wrote:
> On Fri, 6 Nov 2020 at 09:26, Linus Walleij <linus.walleij@linaro•org> wrote:
> >
> > On Fri, Nov 6, 2020 at 8:49 AM Naresh Kamboju <naresh.kamboju@linaro•org> wrote:
> >
> > > arm KASAN build failure noticed on linux next 20201106 tag.
> > > gcc: 9.x
> > >
> > > Build error:
> > > ---------------
> > > arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias'
> > > argument not a string
> > >    24 | void *__memcpy(void *__dest, __const void *__src, size_t __n)
> > > __alias(memcpy);
> > >       | ^~~~
> > > arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias'
> > > argument not a string
> > >    25 | void *__memmove(void *__dest, __const void *__src, size_t
> > > count) __alias(memmove);
> > >       | ^~~~
> > > arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias'
> > > argument not a string
> > >    26 | void *__memset(void *s, int c, size_t count) __alias(memset);
> > >       | ^~~~
> > >
> > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro•org>
> > >
> > > Build details link,
> > > https://builds.tuxbuild.com/1juBs4tXRA6Cwhd1Qnhh4vzCtDx/
> >
> > This looks like a randconfig build.
> >
> > Please drill down and try to report which combination of config
> > options that give rise to this problem so we have a chance of
> > amending it.
> >
> 
> AFAIK there is an incompatible change in -next to change the
> definition of the __alias() macro

Indeed. The following diff needs to be applied as a fixup to
treewide-remove-stringification-from-__alias-macro-definition.patch in
mmotm.

Cheers,
Nathan

diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
index 8c0fa276d994..cc6198f8a348 100644
--- a/arch/arm/boot/compressed/string.c
+++ b/arch/arm/boot/compressed/string.c
@@ -21,9 +21,9 @@
 #undef memcpy
 #undef memmove
 #undef memset
-void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
-void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
-void *__memset(void *s, int c, size_t count) __alias(memset);
+void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias("memcpy");
+void *__memmove(void *__dest, __const void *__src, size_t count) __alias("memmove");
+void *__memset(void *s, int c, size_t count) __alias("memset");
 #endif
 
 void *memcpy(void *__dest, __const void *__src, size_t __n)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06  9:44         ` Nathan Chancellor
@ 2020-11-06 13:37           ` Linus Walleij
  2020-11-06 15:15             ` Russell King - ARM Linux admin
  2020-11-09 16:05             ` Linus Walleij
  0 siblings, 2 replies; 16+ messages in thread
From: Linus Walleij @ 2020-11-06 13:37 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ard Biesheuvel, Naresh Kamboju, Linux-Next Mailing List,
	Florian Fainelli, Abbott Liu, Russell King, Andrey Ryabinin,
	Mike Rapoport, Ahmad Fatoum, Arnd Bergmann, kasan-dev,
	Alexander Potapenko, Linux ARM, Dmitry Vyukov, Stephen Rothwell

On Fri, Nov 6, 2020 at 10:44 AM Nathan Chancellor
<natechancellor@gmail•com> wrote:
> On Fri, Nov 06, 2020 at 09:28:09AM +0100, Ard Biesheuvel wrote:

> > AFAIK there is an incompatible change in -next to change the
> > definition of the __alias() macro
>
> Indeed. The following diff needs to be applied as a fixup to
> treewide-remove-stringification-from-__alias-macro-definition.patch in
> mmotm.
>
> Cheers,
> Nathan
>
> diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> index 8c0fa276d994..cc6198f8a348 100644
> --- a/arch/arm/boot/compressed/string.c
> +++ b/arch/arm/boot/compressed/string.c
> @@ -21,9 +21,9 @@
>  #undef memcpy
>  #undef memmove
>  #undef memset
> -void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> -void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> -void *__memset(void *s, int c, size_t count) __alias(memset);
> +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias("memcpy");
> +void *__memmove(void *__dest, __const void *__src, size_t count) __alias("memmove");
> +void *__memset(void *s, int c, size_t count) __alias("memset");
>  #endif
>
>  void *memcpy(void *__dest, __const void *__src, size_t __n)

Aha. So shall we submit this to Russell? I figure that his git will not
build *without* the changes from mmotm?

That tree isn't using git either is it?

Is this one of those cases where we should ask Stephen R
to carry this patch on top of -next until the merge window?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06 13:37           ` Linus Walleij
@ 2020-11-06 15:15             ` Russell King - ARM Linux admin
  2020-11-06 15:18               ` Ard Biesheuvel
                                 ` (2 more replies)
  2020-11-09 16:05             ` Linus Walleij
  1 sibling, 3 replies; 16+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-06 15:15 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nathan Chancellor, Stephen Rothwell, Florian Fainelli,
	Ahmad Fatoum, Arnd Bergmann, Abbott Liu, Naresh Kamboju,
	kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin,
	Ard Biesheuvel, Linux ARM

On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> On Fri, Nov 6, 2020 at 10:44 AM Nathan Chancellor
> <natechancellor@gmail•com> wrote:
> > On Fri, Nov 06, 2020 at 09:28:09AM +0100, Ard Biesheuvel wrote:
> 
> > > AFAIK there is an incompatible change in -next to change the
> > > definition of the __alias() macro
> >
> > Indeed. The following diff needs to be applied as a fixup to
> > treewide-remove-stringification-from-__alias-macro-definition.patch in
> > mmotm.
> >
> > Cheers,
> > Nathan
> >
> > diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> > index 8c0fa276d994..cc6198f8a348 100644
> > --- a/arch/arm/boot/compressed/string.c
> > +++ b/arch/arm/boot/compressed/string.c
> > @@ -21,9 +21,9 @@
> >  #undef memcpy
> >  #undef memmove
> >  #undef memset
> > -void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> > -void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> > -void *__memset(void *s, int c, size_t count) __alias(memset);
> > +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias("memcpy");
> > +void *__memmove(void *__dest, __const void *__src, size_t count) __alias("memmove");
> > +void *__memset(void *s, int c, size_t count) __alias("memset");
> >  #endif
> >
> >  void *memcpy(void *__dest, __const void *__src, size_t __n)
> 
> Aha. So shall we submit this to Russell? I figure that his git will not
> build *without* the changes from mmotm?
> 
> That tree isn't using git either is it?
> 
> Is this one of those cases where we should ask Stephen R
> to carry this patch on top of -next until the merge window?

Another solution would be to drop 9017/2 ("Enable KASan for ARM")
until the following merge window, and queue up the non-conflicing
ARM KASan fixes in my "misc" branch along with the rest of KASan,
and the conflicting patches along with 9017/2 in the following
merge window.

That means delaying KASan enablement another three months or so,
but should result in less headaches about how to avoid build
breakage with different bits going through different trees.

Comments?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06 15:15             ` Russell King - ARM Linux admin
@ 2020-11-06 15:18               ` Ard Biesheuvel
  2020-11-06 18:09               ` Nathan Chancellor
  2020-11-09 16:02               ` Linus Walleij
  2 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2020-11-06 15:18 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Linus Walleij, Nathan Chancellor, Stephen Rothwell,
	Florian Fainelli, Ahmad Fatoum, Arnd Bergmann, Abbott Liu,
	Naresh Kamboju, kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin, Linux ARM

On Fri, 6 Nov 2020 at 16:16, Russell King - ARM Linux admin
<linux@armlinux•org.uk> wrote:
>
> On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > On Fri, Nov 6, 2020 at 10:44 AM Nathan Chancellor
> > <natechancellor@gmail•com> wrote:
> > > On Fri, Nov 06, 2020 at 09:28:09AM +0100, Ard Biesheuvel wrote:
> >
> > > > AFAIK there is an incompatible change in -next to change the
> > > > definition of the __alias() macro
> > >
> > > Indeed. The following diff needs to be applied as a fixup to
> > > treewide-remove-stringification-from-__alias-macro-definition.patch in
> > > mmotm.
> > >
> > > Cheers,
> > > Nathan
> > >
> > > diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> > > index 8c0fa276d994..cc6198f8a348 100644
> > > --- a/arch/arm/boot/compressed/string.c
> > > +++ b/arch/arm/boot/compressed/string.c
> > > @@ -21,9 +21,9 @@
> > >  #undef memcpy
> > >  #undef memmove
> > >  #undef memset
> > > -void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> > > -void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> > > -void *__memset(void *s, int c, size_t count) __alias(memset);
> > > +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias("memcpy");
> > > +void *__memmove(void *__dest, __const void *__src, size_t count) __alias("memmove");
> > > +void *__memset(void *s, int c, size_t count) __alias("memset");
> > >  #endif
> > >
> > >  void *memcpy(void *__dest, __const void *__src, size_t __n)
> >
> > Aha. So shall we submit this to Russell? I figure that his git will not
> > build *without* the changes from mmotm?
> >
> > That tree isn't using git either is it?
> >
> > Is this one of those cases where we should ask Stephen R
> > to carry this patch on top of -next until the merge window?
>
> Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> until the following merge window, and queue up the non-conflicing
> ARM KASan fixes in my "misc" branch along with the rest of KASan,
> and the conflicting patches along with 9017/2 in the following
> merge window.
>
> That means delaying KASan enablement another three months or so,
> but should result in less headaches about how to avoid build
> breakage with different bits going through different trees.
>
> Comments?
>

Alternatively, we could simply switch these to the bare
__attribute__((alias(".."))) syntax now, and revert that change again
one cycle later.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06 15:15             ` Russell King - ARM Linux admin
  2020-11-06 15:18               ` Ard Biesheuvel
@ 2020-11-06 18:09               ` Nathan Chancellor
  2020-11-09 16:02               ` Linus Walleij
  2 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2020-11-06 18:09 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Linus Walleij, Stephen Rothwell, Florian Fainelli, Ahmad Fatoum,
	Arnd Bergmann, Abbott Liu, Naresh Kamboju, kasan-dev,
	Mike Rapoport, Linux-Next Mailing List, Alexander Potapenko,
	Dmitry Vyukov, Andrey Ryabinin, Ard Biesheuvel, Linux ARM

On Fri, Nov 06, 2020 at 03:15:54PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > On Fri, Nov 6, 2020 at 10:44 AM Nathan Chancellor
> > <natechancellor@gmail•com> wrote:
> > > On Fri, Nov 06, 2020 at 09:28:09AM +0100, Ard Biesheuvel wrote:
> > 
> > > > AFAIK there is an incompatible change in -next to change the
> > > > definition of the __alias() macro
> > >
> > > Indeed. The following diff needs to be applied as a fixup to
> > > treewide-remove-stringification-from-__alias-macro-definition.patch in
> > > mmotm.
> > >
> > > Cheers,
> > > Nathan
> > >
> > > diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> > > index 8c0fa276d994..cc6198f8a348 100644
> > > --- a/arch/arm/boot/compressed/string.c
> > > +++ b/arch/arm/boot/compressed/string.c
> > > @@ -21,9 +21,9 @@
> > >  #undef memcpy
> > >  #undef memmove
> > >  #undef memset
> > > -void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> > > -void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> > > -void *__memset(void *s, int c, size_t count) __alias(memset);
> > > +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias("memcpy");
> > > +void *__memmove(void *__dest, __const void *__src, size_t count) __alias("memmove");
> > > +void *__memset(void *s, int c, size_t count) __alias("memset");
> > >  #endif
> > >
> > >  void *memcpy(void *__dest, __const void *__src, size_t __n)
> > 
> > Aha. So shall we submit this to Russell? I figure that his git will not
> > build *without* the changes from mmotm?

Yeah, I do not think that you can apply that diff to Russell's tree
without the patch from -mm.

> > That tree isn't using git either is it?
> > 
> > Is this one of those cases where we should ask Stephen R
> > to carry this patch on top of -next until the merge window?

I believe so, I do not think Stephen has any issues with carrying that
diff to keep everything building properly (although I won't speak for
him heh).

> Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> until the following merge window, and queue up the non-conflicing
> ARM KASan fixes in my "misc" branch along with the rest of KASan,
> and the conflicting patches along with 9017/2 in the following
> merge window.
> 
> That means delaying KASan enablement another three months or so,
> but should result in less headaches about how to avoid build
> breakage with different bits going through different trees.
> 
> Comments?

That could certainly work but as far as I am aware, that is really the
only breakage. In theory, Andrew could just hold off on sending that
patch until after yours is merged into Linus' tree so that it could be
added to that patch and everything stays building properly. Requires a
minor amount of coordination but that would avoid delaying KASAN
enablement for three months. I do not have any preference since this is
not my code.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06 15:15             ` Russell King - ARM Linux admin
  2020-11-06 15:18               ` Ard Biesheuvel
  2020-11-06 18:09               ` Nathan Chancellor
@ 2020-11-09 16:02               ` Linus Walleij
  2020-11-09 16:06                 ` Russell King - ARM Linux admin
  2 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2020-11-09 16:02 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Nathan Chancellor, Stephen Rothwell, Florian Fainelli,
	Ahmad Fatoum, Arnd Bergmann, Abbott Liu, Naresh Kamboju,
	kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin,
	Ard Biesheuvel, Linux ARM

On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
<linux@armlinux•org.uk> wrote:
> On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:

> > Aha. So shall we submit this to Russell? I figure that his git will not
> > build *without* the changes from mmotm?
> >
> > That tree isn't using git either is it?
> >
> > Is this one of those cases where we should ask Stephen R
> > to carry this patch on top of -next until the merge window?
>
> Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> until the following merge window, and queue up the non-conflicing
> ARM KASan fixes in my "misc" branch along with the rest of KASan,
> and the conflicting patches along with 9017/2 in the following
> merge window.
>
> That means delaying KASan enablement another three months or so,
> but should result in less headaches about how to avoid build
> breakage with different bits going through different trees.
>
> Comments?

I suppose I would survive deferring it. Or we could merge the
smaller enablement patch towards the end of the merge
window once the MM changes are in.

If it is just *one* patch in the MM tree I suppose we could also
just apply that one patch also to the ARM tree, and then this
fixup on top. It does look a bit convoluted in the git history with
two hashes and the same patch twice, but it's what I've done
at times when there was no other choice that doing that or
deferring development. It works as long as the patches are
textually identical: git will cope.
If there is a risk that the patch in MM changes this latter
approach is a no-go.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-06 13:37           ` Linus Walleij
  2020-11-06 15:15             ` Russell King - ARM Linux admin
@ 2020-11-09 16:05             ` Linus Walleij
  1 sibling, 0 replies; 16+ messages in thread
From: Linus Walleij @ 2020-11-09 16:05 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Ard Biesheuvel, Naresh Kamboju, Linux-Next Mailing List,
	Florian Fainelli, Abbott Liu, Russell King, Andrey Ryabinin,
	Mike Rapoport, Ahmad Fatoum, Arnd Bergmann, kasan-dev,
	Alexander Potapenko, Linux ARM, Dmitry Vyukov, Stephen Rothwell

On Fri, Nov 6, 2020 at 2:37 PM Linus Walleij <linus.walleij@linaro•org> wrote:

> Is this one of those cases where we should ask Stephen R
> to carry this patch on top of -next until the merge window?

Apparently this is being handled by "post-next" which I have no
idea how it works, seems like a merge quirk path, but if it works
out, I'm happy :D

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-09 16:02               ` Linus Walleij
@ 2020-11-09 16:06                 ` Russell King - ARM Linux admin
  2020-11-10 12:04                   ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-09 16:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nathan Chancellor, Stephen Rothwell, Florian Fainelli,
	Ahmad Fatoum, Arnd Bergmann, Abbott Liu, Naresh Kamboju,
	kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin,
	Ard Biesheuvel, Linux ARM

On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> <linux@armlinux•org.uk> wrote:
> > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> 
> > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > build *without* the changes from mmotm?
> > >
> > > That tree isn't using git either is it?
> > >
> > > Is this one of those cases where we should ask Stephen R
> > > to carry this patch on top of -next until the merge window?
> >
> > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > until the following merge window, and queue up the non-conflicing
> > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > and the conflicting patches along with 9017/2 in the following
> > merge window.
> >
> > That means delaying KASan enablement another three months or so,
> > but should result in less headaches about how to avoid build
> > breakage with different bits going through different trees.
> >
> > Comments?
> 
> I suppose I would survive deferring it. Or we could merge the
> smaller enablement patch towards the end of the merge
> window once the MM changes are in.
> 
> If it is just *one* patch in the MM tree I suppose we could also
> just apply that one patch also to the ARM tree, and then this
> fixup on top. It does look a bit convoluted in the git history with
> two hashes and the same patch twice, but it's what I've done
> at times when there was no other choice that doing that or
> deferring development. It works as long as the patches are
> textually identical: git will cope.

I thought there was a problem that if I applied the fix then my tree
no longer builds without the changes in -mm?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-09 16:06                 ` Russell King - ARM Linux admin
@ 2020-11-10 12:04                   ` Ard Biesheuvel
  2020-11-12 13:51                     ` Linus Walleij
  0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2020-11-10 12:04 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Linus Walleij, Nathan Chancellor, Stephen Rothwell,
	Florian Fainelli, Ahmad Fatoum, Arnd Bergmann, Abbott Liu,
	Naresh Kamboju, kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin, Linux ARM

On Mon, 9 Nov 2020 at 17:07, Russell King - ARM Linux admin
<linux@armlinux•org.uk> wrote:
>
> On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> > On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> > <linux@armlinux•org.uk> wrote:
> > > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> >
> > > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > > build *without* the changes from mmotm?
> > > >
> > > > That tree isn't using git either is it?
> > > >
> > > > Is this one of those cases where we should ask Stephen R
> > > > to carry this patch on top of -next until the merge window?
> > >
> > > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > > until the following merge window, and queue up the non-conflicing
> > > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > > and the conflicting patches along with 9017/2 in the following
> > > merge window.
> > >
> > > That means delaying KASan enablement another three months or so,
> > > but should result in less headaches about how to avoid build
> > > breakage with different bits going through different trees.
> > >
> > > Comments?
> >
> > I suppose I would survive deferring it. Or we could merge the
> > smaller enablement patch towards the end of the merge
> > window once the MM changes are in.
> >
> > If it is just *one* patch in the MM tree I suppose we could also
> > just apply that one patch also to the ARM tree, and then this
> > fixup on top. It does look a bit convoluted in the git history with
> > two hashes and the same patch twice, but it's what I've done
> > at times when there was no other choice that doing that or
> > deferring development. It works as long as the patches are
> > textually identical: git will cope.
>
> I thought there was a problem that if I applied the fix then my tree
> no longer builds without the changes in -mm?
>

Indeed. Someone is changing the __alias() wrappers [for no good reason
afaict] in a way that does not allow for new users of those wrappers
to come in concurrently.

Hency my suggestion to switch to the raw __attribute__((alias("..")))
notation for the time being, and switch back to __alias() somewhere
after v5.11-rc1.

Or we might add this to the file in question

#undef __alias
#define __alias(symbol) __attribute__((__alias__(symbol)))

and switch to the quoted versions of the identifier. Then we can just
drop these two lines again later, after v5.11-rc1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-10 12:04                   ` Ard Biesheuvel
@ 2020-11-12 13:51                     ` Linus Walleij
  2020-11-12 15:05                       ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2020-11-12 13:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Russell King - ARM Linux admin, Nathan Chancellor,
	Stephen Rothwell, Florian Fainelli, Ahmad Fatoum, Arnd Bergmann,
	Abbott Liu, Naresh Kamboju, kasan-dev, Mike Rapoport,
	Linux-Next Mailing List, Alexander Potapenko, Dmitry Vyukov,
	Andrey Ryabinin, Linux ARM

On Tue, Nov 10, 2020 at 1:05 PM Ard Biesheuvel <ardb@kernel•org> wrote:
> On Mon, 9 Nov 2020 at 17:07, Russell King - ARM Linux admin
> <linux@armlinux•org.uk> wrote:
> >
> > On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> > > On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> > > <linux@armlinux•org.uk> wrote:
> > > > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > >
> > > > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > > > build *without* the changes from mmotm?
> > > > >
> > > > > That tree isn't using git either is it?
> > > > >
> > > > > Is this one of those cases where we should ask Stephen R
> > > > > to carry this patch on top of -next until the merge window?
> > > >
> > > > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > > > until the following merge window, and queue up the non-conflicing
> > > > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > > > and the conflicting patches along with 9017/2 in the following
> > > > merge window.
> > > >
> > > > That means delaying KASan enablement another three months or so,
> > > > but should result in less headaches about how to avoid build
> > > > breakage with different bits going through different trees.
> > > >
> > > > Comments?
> > >
> > > I suppose I would survive deferring it. Or we could merge the
> > > smaller enablement patch towards the end of the merge
> > > window once the MM changes are in.
> > >
> > > If it is just *one* patch in the MM tree I suppose we could also
> > > just apply that one patch also to the ARM tree, and then this
> > > fixup on top. It does look a bit convoluted in the git history with
> > > two hashes and the same patch twice, but it's what I've done
> > > at times when there was no other choice that doing that or
> > > deferring development. It works as long as the patches are
> > > textually identical: git will cope.
> >
> > I thought there was a problem that if I applied the fix then my tree
> > no longer builds without the changes in -mm?
> >
>
> Indeed. Someone is changing the __alias() wrappers [for no good reason
> afaict] in a way that does not allow for new users of those wrappers
> to come in concurrently.
>
> Hency my suggestion to switch to the raw __attribute__((alias("..")))
> notation for the time being, and switch back to __alias() somewhere
> after v5.11-rc1.
>
> Or we might add this to the file in question
>
> #undef __alias
> #define __alias(symbol) __attribute__((__alias__(symbol)))
>
> and switch to the quoted versions of the identifier. Then we can just
> drop these two lines again later, after v5.11-rc1

I was under the impression that there was some "post-next"
trick that mmot apply this patch after -next has been merged
so it's solved now?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-12 13:51                     ` Linus Walleij
@ 2020-11-12 15:05                       ` Ard Biesheuvel
  2020-11-12 17:52                         ` Nathan Chancellor
  0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2020-11-12 15:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King - ARM Linux admin, Nathan Chancellor,
	Stephen Rothwell, Florian Fainelli, Ahmad Fatoum, Arnd Bergmann,
	Abbott Liu, Naresh Kamboju, kasan-dev, Mike Rapoport,
	Linux-Next Mailing List, Alexander Potapenko, Dmitry Vyukov,
	Andrey Ryabinin, Linux ARM

On Thu, 12 Nov 2020 at 14:51, Linus Walleij <linus.walleij@linaro•org> wrote:
>
> On Tue, Nov 10, 2020 at 1:05 PM Ard Biesheuvel <ardb@kernel•org> wrote:
> > On Mon, 9 Nov 2020 at 17:07, Russell King - ARM Linux admin
> > <linux@armlinux•org.uk> wrote:
> > >
> > > On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> > > > On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> > > > <linux@armlinux•org.uk> wrote:
> > > > > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > > >
> > > > > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > > > > build *without* the changes from mmotm?
> > > > > >
> > > > > > That tree isn't using git either is it?
> > > > > >
> > > > > > Is this one of those cases where we should ask Stephen R
> > > > > > to carry this patch on top of -next until the merge window?
> > > > >
> > > > > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > > > > until the following merge window, and queue up the non-conflicing
> > > > > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > > > > and the conflicting patches along with 9017/2 in the following
> > > > > merge window.
> > > > >
> > > > > That means delaying KASan enablement another three months or so,
> > > > > but should result in less headaches about how to avoid build
> > > > > breakage with different bits going through different trees.
> > > > >
> > > > > Comments?
> > > >
> > > > I suppose I would survive deferring it. Or we could merge the
> > > > smaller enablement patch towards the end of the merge
> > > > window once the MM changes are in.
> > > >
> > > > If it is just *one* patch in the MM tree I suppose we could also
> > > > just apply that one patch also to the ARM tree, and then this
> > > > fixup on top. It does look a bit convoluted in the git history with
> > > > two hashes and the same patch twice, but it's what I've done
> > > > at times when there was no other choice that doing that or
> > > > deferring development. It works as long as the patches are
> > > > textually identical: git will cope.
> > >
> > > I thought there was a problem that if I applied the fix then my tree
> > > no longer builds without the changes in -mm?
> > >
> >
> > Indeed. Someone is changing the __alias() wrappers [for no good reason
> > afaict] in a way that does not allow for new users of those wrappers
> > to come in concurrently.
> >
> > Hency my suggestion to switch to the raw __attribute__((alias("..")))
> > notation for the time being, and switch back to __alias() somewhere
> > after v5.11-rc1.
> >
> > Or we might add this to the file in question
> >
> > #undef __alias
> > #define __alias(symbol) __attribute__((__alias__(symbol)))
> >
> > and switch to the quoted versions of the identifier. Then we can just
> > drop these two lines again later, after v5.11-rc1
>
> I was under the impression that there was some "post-next"
> trick that mmot apply this patch after -next has been merged
> so it's solved now?
>

Yes, it appears that [0] has been picked up, I guess we weren't cc'ed
on the version that was sent to akpm [which is fine btw, although a
followup reply here that things are all good now would have been
appreciated]


https://lore.kernel.org/linux-arm-kernel/20201109001712.3384097-1-natechancellor@gmail.com/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-12 15:05                       ` Ard Biesheuvel
@ 2020-11-12 17:52                         ` Nathan Chancellor
  2020-11-16 15:16                           ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Nathan Chancellor @ 2020-11-12 17:52 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linus Walleij, Russell King - ARM Linux admin, Stephen Rothwell,
	Florian Fainelli, Ahmad Fatoum, Arnd Bergmann, Abbott Liu,
	Naresh Kamboju, kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin, Linux ARM

On Thu, Nov 12, 2020 at 04:05:52PM +0100, Ard Biesheuvel wrote:
> On Thu, 12 Nov 2020 at 14:51, Linus Walleij <linus.walleij@linaro•org> wrote:
> >
> > On Tue, Nov 10, 2020 at 1:05 PM Ard Biesheuvel <ardb@kernel•org> wrote:
> > > On Mon, 9 Nov 2020 at 17:07, Russell King - ARM Linux admin
> > > <linux@armlinux•org.uk> wrote:
> > > >
> > > > On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> > > > > On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> > > > > <linux@armlinux•org.uk> wrote:
> > > > > > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > > > >
> > > > > > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > > > > > build *without* the changes from mmotm?
> > > > > > >
> > > > > > > That tree isn't using git either is it?
> > > > > > >
> > > > > > > Is this one of those cases where we should ask Stephen R
> > > > > > > to carry this patch on top of -next until the merge window?
> > > > > >
> > > > > > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > > > > > until the following merge window, and queue up the non-conflicing
> > > > > > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > > > > > and the conflicting patches along with 9017/2 in the following
> > > > > > merge window.
> > > > > >
> > > > > > That means delaying KASan enablement another three months or so,
> > > > > > but should result in less headaches about how to avoid build
> > > > > > breakage with different bits going through different trees.
> > > > > >
> > > > > > Comments?
> > > > >
> > > > > I suppose I would survive deferring it. Or we could merge the
> > > > > smaller enablement patch towards the end of the merge
> > > > > window once the MM changes are in.
> > > > >
> > > > > If it is just *one* patch in the MM tree I suppose we could also
> > > > > just apply that one patch also to the ARM tree, and then this
> > > > > fixup on top. It does look a bit convoluted in the git history with
> > > > > two hashes and the same patch twice, but it's what I've done
> > > > > at times when there was no other choice that doing that or
> > > > > deferring development. It works as long as the patches are
> > > > > textually identical: git will cope.
> > > >
> > > > I thought there was a problem that if I applied the fix then my tree
> > > > no longer builds without the changes in -mm?
> > > >
> > >
> > > Indeed. Someone is changing the __alias() wrappers [for no good reason
> > > afaict] in a way that does not allow for new users of those wrappers
> > > to come in concurrently.
> > >
> > > Hency my suggestion to switch to the raw __attribute__((alias("..")))
> > > notation for the time being, and switch back to __alias() somewhere
> > > after v5.11-rc1.
> > >
> > > Or we might add this to the file in question
> > >
> > > #undef __alias
> > > #define __alias(symbol) __attribute__((__alias__(symbol)))
> > >
> > > and switch to the quoted versions of the identifier. Then we can just
> > > drop these two lines again later, after v5.11-rc1
> >
> > I was under the impression that there was some "post-next"
> > trick that mmot apply this patch after -next has been merged
> > so it's solved now?
> >
> 
> Yes, it appears that [0] has been picked up, I guess we weren't cc'ed
> on the version that was sent to akpm [which is fine btw, although a
> followup reply here that things are all good now would have been
> appreciated]
> 
> 
> https://lore.kernel.org/linux-arm-kernel/20201109001712.3384097-1-natechancellor@gmail.com/

Hi Ard,

Odd, you were on the list of people to receive that patch and you acked
it but it seems that Andrew did not CC you when he actually applied the
patch:

https://lore.kernel.org/mm-commits/20201110212436.yGYhesom8%25akpm@linux-foundation.org/

My apologies for not following up, we appear to be all good now for the
time being (aside from the futex issue that I reported earlier).

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan
  2020-11-12 17:52                         ` Nathan Chancellor
@ 2020-11-16 15:16                           ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2020-11-16 15:16 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Linus Walleij, Russell King - ARM Linux admin, Stephen Rothwell,
	Florian Fainelli, Ahmad Fatoum, Arnd Bergmann, Abbott Liu,
	Naresh Kamboju, kasan-dev, Mike Rapoport, Linux-Next Mailing List,
	Alexander Potapenko, Dmitry Vyukov, Andrey Ryabinin, Linux ARM

On Thu, 12 Nov 2020 at 18:52, Nathan Chancellor
<natechancellor@gmail•com> wrote:
>
> On Thu, Nov 12, 2020 at 04:05:52PM +0100, Ard Biesheuvel wrote:
> > On Thu, 12 Nov 2020 at 14:51, Linus Walleij <linus.walleij@linaro•org> wrote:
> > >
> > > On Tue, Nov 10, 2020 at 1:05 PM Ard Biesheuvel <ardb@kernel•org> wrote:
> > > > On Mon, 9 Nov 2020 at 17:07, Russell King - ARM Linux admin
> > > > <linux@armlinux•org.uk> wrote:
> > > > >
> > > > > On Mon, Nov 09, 2020 at 05:02:09PM +0100, Linus Walleij wrote:
> > > > > > On Fri, Nov 6, 2020 at 4:16 PM Russell King - ARM Linux admin
> > > > > > <linux@armlinux•org.uk> wrote:
> > > > > > > On Fri, Nov 06, 2020 at 02:37:21PM +0100, Linus Walleij wrote:
> > > > > >
> > > > > > > > Aha. So shall we submit this to Russell? I figure that his git will not
> > > > > > > > build *without* the changes from mmotm?
> > > > > > > >
> > > > > > > > That tree isn't using git either is it?
> > > > > > > >
> > > > > > > > Is this one of those cases where we should ask Stephen R
> > > > > > > > to carry this patch on top of -next until the merge window?
> > > > > > >
> > > > > > > Another solution would be to drop 9017/2 ("Enable KASan for ARM")
> > > > > > > until the following merge window, and queue up the non-conflicing
> > > > > > > ARM KASan fixes in my "misc" branch along with the rest of KASan,
> > > > > > > and the conflicting patches along with 9017/2 in the following
> > > > > > > merge window.
> > > > > > >
> > > > > > > That means delaying KASan enablement another three months or so,
> > > > > > > but should result in less headaches about how to avoid build
> > > > > > > breakage with different bits going through different trees.
> > > > > > >
> > > > > > > Comments?
> > > > > >
> > > > > > I suppose I would survive deferring it. Or we could merge the
> > > > > > smaller enablement patch towards the end of the merge
> > > > > > window once the MM changes are in.
> > > > > >
> > > > > > If it is just *one* patch in the MM tree I suppose we could also
> > > > > > just apply that one patch also to the ARM tree, and then this
> > > > > > fixup on top. It does look a bit convoluted in the git history with
> > > > > > two hashes and the same patch twice, but it's what I've done
> > > > > > at times when there was no other choice that doing that or
> > > > > > deferring development. It works as long as the patches are
> > > > > > textually identical: git will cope.
> > > > >
> > > > > I thought there was a problem that if I applied the fix then my tree
> > > > > no longer builds without the changes in -mm?
> > > > >
> > > >
> > > > Indeed. Someone is changing the __alias() wrappers [for no good reason
> > > > afaict] in a way that does not allow for new users of those wrappers
> > > > to come in concurrently.
> > > >
> > > > Hency my suggestion to switch to the raw __attribute__((alias("..")))
> > > > notation for the time being, and switch back to __alias() somewhere
> > > > after v5.11-rc1.
> > > >
> > > > Or we might add this to the file in question
> > > >
> > > > #undef __alias
> > > > #define __alias(symbol) __attribute__((__alias__(symbol)))
> > > >
> > > > and switch to the quoted versions of the identifier. Then we can just
> > > > drop these two lines again later, after v5.11-rc1
> > >
> > > I was under the impression that there was some "post-next"
> > > trick that mmot apply this patch after -next has been merged
> > > so it's solved now?
> > >
> >
> > Yes, it appears that [0] has been picked up, I guess we weren't cc'ed
> > on the version that was sent to akpm [which is fine btw, although a
> > followup reply here that things are all good now would have been
> > appreciated]
> >
> >
> > https://lore.kernel.org/linux-arm-kernel/20201109001712.3384097-1-natechancellor@gmail.com/
>
> Hi Ard,
>
> Odd, you were on the list of people to receive that patch and you acked
> it but it seems that Andrew did not CC you when he actually applied the
> patch:
>
> https://lore.kernel.org/mm-commits/20201110212436.yGYhesom8%25akpm@linux-foundation.org/
>
> My apologies for not following up, we appear to be all good now for the
> time being (aside from the futex issue that I reported earlier).
>

No worries - at least it is fixed now. And KASAN is already shaking
out bugs, so it is great that we finally managed to enable this for
ARM.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-11-16 15:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20201019084140.4532-1-linus.walleij@linaro.org>
     [not found] ` <20201019084140.4532-3-linus.walleij@linaro.org>
2020-11-06  7:49   ` [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan Naresh Kamboju
2020-11-06  8:26     ` Linus Walleij
2020-11-06  8:28       ` Ard Biesheuvel
2020-11-06  9:44         ` Nathan Chancellor
2020-11-06 13:37           ` Linus Walleij
2020-11-06 15:15             ` Russell King - ARM Linux admin
2020-11-06 15:18               ` Ard Biesheuvel
2020-11-06 18:09               ` Nathan Chancellor
2020-11-09 16:02               ` Linus Walleij
2020-11-09 16:06                 ` Russell King - ARM Linux admin
2020-11-10 12:04                   ` Ard Biesheuvel
2020-11-12 13:51                     ` Linus Walleij
2020-11-12 15:05                       ` Ard Biesheuvel
2020-11-12 17:52                         ` Nathan Chancellor
2020-11-16 15:16                           ` Ard Biesheuvel
2020-11-09 16:05             ` Linus Walleij

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