From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Subject: Re: [PATCH v4 05/11] arm64: csum: Disable KASAN for do_csum() Date: Wed, 22 Apr 2020 10:49:52 +0100 Message-ID: <20200422094951.GA54428@lakrids.cambridge.arm.com> References: <20200421151537.19241-1-will@kernel.org> <20200421151537.19241-6-will@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20200421151537.19241-6-will@kernel.org> Sender: linux-kernel-owner@vger.kernel.org To: Will Deacon Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, kernel-team@android.com, Michael Ellerman , Peter Zijlstra , Linus Torvalds , Segher Boessenkool , Christian Borntraeger , Luc Van Oostenryck , Arnd Bergmann , Peter Oberparleiter , Masahiro Yamada , Nick Desaulniers , Robin Murphy List-Id: linux-arch.vger.kernel.org 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 > Cc: Robin Murphy > Signed-off-by: Will Deacon >From a functional perspective: Acked-by: Mark Rutland 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. Thanks, Mark. > --- > arch/arm64/lib/csum.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/lib/csum.c b/arch/arm64/lib/csum.c > index 60eccae2abad..78b87a64ca0a 100644 > --- a/arch/arm64/lib/csum.c > +++ b/arch/arm64/lib/csum.c > @@ -14,7 +14,11 @@ static u64 accumulate(u64 sum, u64 data) > return tmp + (tmp >> 64); > } > > -unsigned int do_csum(const unsigned char *buff, int len) > +/* > + * We over-read the buffer and this makes KASAN unhappy. Instead, disable > + * instrumentation and call kasan explicitly. > + */ > +unsigned int __no_sanitize_address do_csum(const unsigned char *buff, int len) > { > unsigned int offset, shift, sum; > const u64 *ptr; > @@ -42,7 +46,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > * odd/even alignment, and means we can ignore it until the very end. > */ > shift = offset * 8; > - data = READ_ONCE_NOCHECK(*ptr++); > + data = *ptr++; > #ifdef __LITTLE_ENDIAN > data = (data >> shift) << shift; > #else > @@ -58,10 +62,10 @@ unsigned int do_csum(const unsigned char *buff, int len) > while (unlikely(len > 64)) { > __uint128_t tmp1, tmp2, tmp3, tmp4; > > - tmp1 = READ_ONCE_NOCHECK(*(__uint128_t *)ptr); > - tmp2 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 2)); > - tmp3 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 4)); > - tmp4 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 6)); > + tmp1 = *(__uint128_t *)ptr; > + tmp2 = *(__uint128_t *)(ptr + 2); > + tmp3 = *(__uint128_t *)(ptr + 4); > + tmp4 = *(__uint128_t *)(ptr + 6); > > len -= 64; > ptr += 8; > @@ -85,7 +89,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > __uint128_t tmp; > > sum64 = accumulate(sum64, data); > - tmp = READ_ONCE_NOCHECK(*(__uint128_t *)ptr); > + tmp = *(__uint128_t *)ptr; > > len -= 16; > ptr += 2; > @@ -100,7 +104,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > } > if (len > 0) { > sum64 = accumulate(sum64, data); > - data = READ_ONCE_NOCHECK(*ptr); > + data = *ptr; > len -= 8; > } > /* > -- > 2.26.1.301.g55bc3eb7cb9-goog > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com ([217.140.110.172]:45258 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725924AbgDVJuC (ORCPT ); Wed, 22 Apr 2020 05:50:02 -0400 Date: Wed, 22 Apr 2020 10:49:52 +0100 From: Mark Rutland Subject: Re: [PATCH v4 05/11] arm64: csum: Disable KASAN for do_csum() Message-ID: <20200422094951.GA54428@lakrids.cambridge.arm.com> References: <20200421151537.19241-1-will@kernel.org> <20200421151537.19241-6-will@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200421151537.19241-6-will@kernel.org> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Will Deacon Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, kernel-team@android.com, Michael Ellerman , Peter Zijlstra , Linus Torvalds , Segher Boessenkool , Christian Borntraeger , Luc Van Oostenryck , Arnd Bergmann , Peter Oberparleiter , Masahiro Yamada , Nick Desaulniers , Robin Murphy Message-ID: <20200422094952.-2kBxYLnTIKBOv_5gLSctJavNN_DY8BBTItFVaoq3-4@z> 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 > Cc: Robin Murphy > Signed-off-by: Will Deacon >From a functional perspective: Acked-by: Mark Rutland 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. Thanks, Mark. > --- > arch/arm64/lib/csum.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/lib/csum.c b/arch/arm64/lib/csum.c > index 60eccae2abad..78b87a64ca0a 100644 > --- a/arch/arm64/lib/csum.c > +++ b/arch/arm64/lib/csum.c > @@ -14,7 +14,11 @@ static u64 accumulate(u64 sum, u64 data) > return tmp + (tmp >> 64); > } > > -unsigned int do_csum(const unsigned char *buff, int len) > +/* > + * We over-read the buffer and this makes KASAN unhappy. Instead, disable > + * instrumentation and call kasan explicitly. > + */ > +unsigned int __no_sanitize_address do_csum(const unsigned char *buff, int len) > { > unsigned int offset, shift, sum; > const u64 *ptr; > @@ -42,7 +46,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > * odd/even alignment, and means we can ignore it until the very end. > */ > shift = offset * 8; > - data = READ_ONCE_NOCHECK(*ptr++); > + data = *ptr++; > #ifdef __LITTLE_ENDIAN > data = (data >> shift) << shift; > #else > @@ -58,10 +62,10 @@ unsigned int do_csum(const unsigned char *buff, int len) > while (unlikely(len > 64)) { > __uint128_t tmp1, tmp2, tmp3, tmp4; > > - tmp1 = READ_ONCE_NOCHECK(*(__uint128_t *)ptr); > - tmp2 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 2)); > - tmp3 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 4)); > - tmp4 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 6)); > + tmp1 = *(__uint128_t *)ptr; > + tmp2 = *(__uint128_t *)(ptr + 2); > + tmp3 = *(__uint128_t *)(ptr + 4); > + tmp4 = *(__uint128_t *)(ptr + 6); > > len -= 64; > ptr += 8; > @@ -85,7 +89,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > __uint128_t tmp; > > sum64 = accumulate(sum64, data); > - tmp = READ_ONCE_NOCHECK(*(__uint128_t *)ptr); > + tmp = *(__uint128_t *)ptr; > > len -= 16; > ptr += 2; > @@ -100,7 +104,7 @@ unsigned int do_csum(const unsigned char *buff, int len) > } > if (len > 0) { > sum64 = accumulate(sum64, data); > - data = READ_ONCE_NOCHECK(*ptr); > + data = *ptr; > len -= 8; > } > /* > -- > 2.26.1.301.g55bc3eb7cb9-goog >