Re: [PATCH v4 05/11] arm64: csum: Disable KASAN for do_csum()

From: Robin Murphy
Date: Wed Apr 22 2020 - 07:02:08 EST


On 2020-04-22 11:41 am, Will Deacon wrote:
On Wed, Apr 22, 2020 at 10:49:52AM +0100, Mark Rutland wrote:
On Tue, Apr 21, 2020 at 04:15:31PM +0100, Will Deacon wrote:
do_csum() over-reads the source buffer and therefore abuses
READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for
READ_ONCE_NOCHECK() becoming a macro, and therefore losing its
'__no_sanitize_address' annotation, just annotate do_csum() explicitly
and fall back to normal loads.

Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Robin Murphy <robin.murphy@xxxxxxx>
Signed-off-by: Will Deacon <will@xxxxxxxxxx>

From a functional perspective:

Acked-by: Mark Rutland <mark.rutland@xxxxxxx>

Thanks.

I know that Robin had a concern w.r.t. how this would affect the
codegen, but I think we can follow that up after the series as a whole
is merged.

Makes sense. I did look at the codegen, fwiw, and it didn't seem especially
bad. One of the LDP's gets cracked in the unlikely() path, but it didn't
look like it would be a disaster (and sprinkling barrier() around to force
the LDP felt really fragile!).

Sure - I have a nagging feeling that it could still do better WRT pipelining the loads anyway, so I'm happy to come back and reconsider the local codegen later. It certainly doesn't deserve to stand in the way of cross-arch rework.

Other than dereferencing the ptr argument, this code has no cause to make any explicit memory accesses of its own, so I don't think we lose any practical KASAN coverage by moving the annotation to function level. Given all that,

Acked-by: Robin Murphy <robin.murphy@xxxxxxx>

Cheers,
Robin.