[PATCH] ARM: lib64: Properly handle unaligned addresses in string functions

[PATCH] ARM: lib64: Properly handle unaligned addresses in string functions

[Andrey Smirnov]

Together FEC driver and parts of IP stack might end up trying to
memcpy() small chunks of memory from uncached (that is Device memory)
addresses that are not properly aligned, leading to data abort.

To prevent such cases, add code to guard unaligned accesses and
redirect them to byte-wise implementations which do not have the above
problem.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/lib64/string.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib64/string.c b/arch/arm/lib64/string.c
index cb2633152..1b04f4487 100644
--- a/arch/arm/lib64/string.c
+++ b/arch/arm/lib64/string.c
@@ -5,9 +5,16 @@
 void *__arch_memset(void *dst, int c, __kernel_size_t size);
 void *__arch_memcpy(void * dest, const void *src, size_t count);
 
+static bool properly_aligned(const void *ptr)
+{
+	const unsigned long address = (unsigned long)ptr;
+
+	return get_cr() & CR_M && IS_ALIGNED(address, 16);
+}
+
 void *memset(void *dst, int c, __kernel_size_t size)
 {
-	if (likely(get_cr() & CR_M))
+	if (likely(properly_aligned(dst)))
 		return __arch_memset(dst, c, size);
 
 	return __default_memset(dst, c, size);
@@ -15,7 +22,7 @@ void *memset(void *dst, int c, __kernel_size_t size)
 
 void *memcpy(void * dest, const void *src, size_t count)
 {
-	if (likely(get_cr() & CR_M))
+	if (likely(properly_aligned(dest) && properly_aligned(src)))
 		return __arch_memcpy(dest, src, count);
 
 	return __default_memcpy(dest, src, count);
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: lib64: Properly handle unaligned addresses in string functions

[Sascha Hauer]

On Fri, Aug 10, 2018 at 11:52:01AM -0700, Andrey Smirnov wrote:
> Together FEC driver and parts of IP stack might end up trying to
> memcpy() small chunks of memory from uncached (that is Device memory)
> addresses that are not properly aligned, leading to data abort.
> 
> To prevent such cases, add code to guard unaligned accesses and
> redirect them to byte-wise implementations which do not have the above
> problem.

Please no. IMO this problem only shows that the FEC driver shouldn't use
dma_alloc_coherent() to allocate its receive buffers, but instead should
use dma_alloc + dma_sync_*.

Sascha

> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/lib64/string.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/lib64/string.c b/arch/arm/lib64/string.c
> index cb2633152..1b04f4487 100644
> --- a/arch/arm/lib64/string.c
> +++ b/arch/arm/lib64/string.c
> @@ -5,9 +5,16 @@
>  void *__arch_memset(void *dst, int c, __kernel_size_t size);
>  void *__arch_memcpy(void * dest, const void *src, size_t count);
>  
> +static bool properly_aligned(const void *ptr)
> +{
> +	const unsigned long address = (unsigned long)ptr;
> +
> +	return get_cr() & CR_M && IS_ALIGNED(address, 16);
> +}
> +
>  void *memset(void *dst, int c, __kernel_size_t size)
>  {
> -	if (likely(get_cr() & CR_M))
> +	if (likely(properly_aligned(dst)))
>  		return __arch_memset(dst, c, size);
>  
>  	return __default_memset(dst, c, size);
> @@ -15,7 +22,7 @@ void *memset(void *dst, int c, __kernel_size_t size)
>  
>  void *memcpy(void * dest, const void *src, size_t count)
>  {
> -	if (likely(get_cr() & CR_M))
> +	if (likely(properly_aligned(dest) && properly_aligned(src)))
>  		return __arch_memcpy(dest, src, count);
>  
>  	return __default_memcpy(dest, src, count);
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: lib64: Properly handle unaligned addresses in string functions

[Andrey Smirnov]

On Sun, Aug 12, 2018 at 11:41 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Fri, Aug 10, 2018 at 11:52:01AM -0700, Andrey Smirnov wrote:
> > Together FEC driver and parts of IP stack might end up trying to
> > memcpy() small chunks of memory from uncached (that is Device memory)
> > addresses that are not properly aligned, leading to data abort.
> >
> > To prevent such cases, add code to guard unaligned accesses and
> > redirect them to byte-wise implementations which do not have the above
> > problem.
>
> Please no. IMO this problem only shows that the FEC driver shouldn't use
> dma_alloc_coherent() to allocate its receive buffers, but instead should
> use dma_alloc + dma_sync_*.

OK, will work on that approach in v2.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox