[U-Boot] Issue with memcpy when booting a fitImage on SPEAr600

[Quentin Schulz]

Hi all,

On Thu, Dec 06, 2018 at 11:37:14AM +0100, Thomas Petazzoni wrote:
> Hello Stefan,
> 
> On Wed, 5 Dec 2018 12:30:57 +0100, Stefan Roese wrote:
> 
> > > It's weird to me that it's happening with this SoC because it's based on
> > > ARM926ejs which is widely used I assume. Shouldn't have anyone already
> > > encountered the bug? Or is nobody actually booting a fitImage and had the
> > > luck to never call memcpy with src == dest anywhere else in the code?  
> > 
> > I did some work on the SPEAr600 some years ago and I'm pretty sure that
> > I never used FIT image at that time. Sorry, but I can't remember any
> > similar issues like these.
> 
> Well, the issue is in generic ARM code, so whether it's SPEAr600 or any
> other ARM SoC should not really matter here.
> 
> > FWIW, I would not oppose to having at least this "src == dest" check
> > in the code again, since it doesn't really make sense to overwrite
> > an area with the same value - other than for testing purposes.
> 
> The thing is that the memcpy() that gets called does have this src ==
> dest check, as its code starts with:
> 
> ENTRY(memcpy)
>                 cmp     r0, r1
>                 bxeq    lr
> 
> which, if my assembly-fu is not bad, means: if first argument == second
> argument, then return. But even with this src == dest check within
> memcpy() itself, Quentin is seeing memmove() hang when src == dest.
> 
> Another thing is that the memmove() code looks like this:
> 
> {
>         char *tmp, *s;
> 
>         if (dest <= src) {
>                 memcpy(dest, src, count);
> 
> However, having dest <= src does not guarantee that the destination and
> source areas are non-overlapping. And the normal semantic for memcpy()
> is that it doesn't work for overlapping areas. From memcpy(3):
> 
>   The memcpy() function copies n bytes from memory area src to memory
>   area dest.  The memory areas must not overlap.  Use memmove(3) if the
>   memory areas do overlap.
> 
> Of course, this man page documents the memcpy() implementation from the
> C library, and perhaps the semantic of U-Boot memcpy is different.
> 
> So is it correct to use memcpy() to implement memmove() when the areas
> are overlapping ?
> 
> Perhaps Simon Glass, who did the change to use memcpy() in
> cb0eae8cf8aaca76910dee4c7eb536d0814d1bd2 could comment on that ?
> 

Kindly pinging on the question here.

Best regards,
Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190107/49a3b7a0/attachment.sig>