Subject: Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

From: Catalin Marinas <[email-masked]>
Date: Fri, 10 Sep 2021 12:36:26 +0100

Hi Peter,

On Thu, Sep 09, 2021 at 05:35:54PM -0700, Peter Collingbourne wrote:
> (apologies for breaking the threading, I wasn't subscribed to
> linux-arm-kernel when you sent this)

lore.kernel.org has some instructions on how to download the message and
reply to it.

> It looks like this patch breaks the KASAN unit tests when running in
> HW tags mode. You can construct a config that reproduces the problem
> with something like:
> 
> make O=out defconfig
> scripts/config --file out/.config -e CONFIG_KUNIT -e CONFIG_KASAN -e
> CONFIG_KASAN_HW_TAGS -e CONFIG_KASAN_KUNIT_TEST
> yes '' | make O=out syncconfig
> 
> With that the "kmalloc_memmove_invalid_size" test fails and causes the
> kernel panic below.
> 
> What appears to be going on is that whereas the old memcpy
> implementation ends up returning early after not copying very much
> data if supplied a "negative" size as a side effect of using
> conditional branches that implement signed comparisons (e.g. b.ge on
> line 75), the new implementation does not exit early and attempts to
> copy a large amount of data backwards 4 bytes (after having disabled
> MTE early on as a result of a tag mismatch) until it hits a read-only
> page and causes a panic.

Treating the size argument as unsigned long is the correct way, it
matches the function prototype.

> I'm not sure what the correct fix should be. It seems that at least
> when KASAN is enabled we should be able to catch these invalid memcpys
> somehow, print an error report and ideally abort the entire operation.
> But we should do so without causing a performance impact in the usual
> case.

I'm not convinced that's a valid test. Do we have any statement
somewhere on what a valid size for memmove/memcpy is? We could
artificially limit it to VA_BITS or even to 63 bits so that it stays a
positive number but the generic memmove() implementation doesn't do such
checks either.

Since the memcpy/memmove routines don't have a fail-safe mode, the only
thing the kernel can do on an MTE fault is to disable tag checking,
report it and continue. What you need instead is copy_*_kernel_nofault
in the kasan tests as they return the number of bytes copied.

My suggestion for a quick fix would be to remove the kasan test.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel