Re: [Xen-devel] [PATCH for-4.10] string: fix memmove when size is 0

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/10/17 13:03, Roger Pau Monne wrote:
> ubsan in clang 5.0 complains with:
>
> (XEN) UBSAN: Undefined behaviour in string.c:50:28
> (XEN) pointer overflow:
> (XEN) addition of unsigned offset to ffff830000100000 overflowed to 
> ffff8300000fffff
> [...]
> (XEN) Xen call trace:
> (XEN)    [<ffff82d0802dce0d>] ubsan.c#ubsan_epilogue+0xd/0xc0
> (XEN)    [<ffff82d0802de145>] __ubsan_handle_pointer_overflow+0xa5/0xe0
> (XEN)    [<ffff82d0803bf627>] memmove+0x67/0x80
> (XEN)    [<ffff82d080679f87>] page_alloc.c#bootmem_region_add+0x157/0x220
> (XEN)    [<ffff82d080679c66>] init_boot_pages+0x56/0x220
> (XEN)    [<ffff82d0806bcb9d>] __start_xen+0x2c2d/0x4b50
> (XEN)    [<ffff82d0802000f3>] __high_start+0x53/0x60
>
> This is due to memmove doing a n-1+addr when n is 0. This is harmless
> because the loop is bounded to 0. Fix this by returning earlier when n
> is 0.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

There are many passed values which could trigger this warning.  Does

diff --git a/xen/arch/x86/string.c b/xen/arch/x86/string.c
index cd85a38..4f55856 100644
--- a/xen/arch/x86/string.c
+++ b/xen/arch/x86/string.c
@@ -47,7 +47,7 @@ void *(memmove)(void *dest, const void *src, size_t n)
         "   rep movsb   ; "
         "   cld           "
         : "=&c" (d0), "=&S" (d1), "=&D" (d2)
-        : "0" (n), "1" (n-1+(const char *)src), "2" (n-1+(char *)dest)
+        : "0" (n), "1" ((uintptr_t)src + n - 1), "2" ((uintptr_t)dest + n - 1)
         : "memory");
 
     return dest;

work any better?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel