[U-Boot] [PATCH 3/8] sandbox: Use memcpy() to move overlapping regions

[Heinrich Schuchardt]

On 06/09/2018 08:22 PM, Simon Glass wrote:
> The use of strcpy() to remove characters at the start of a string is safe
> in U-Boot, since we know the implementation. But in os.c we are using the
> C library's strcpy() function, where this behaviour is not permitted.
> 
> Update the code to use memcpy() instead.
> 
> Reported-by: Coverity (CID: 173279)
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>  arch/sandbox/cpu/os.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> index 5839932b00..496a8f9bd8 100644
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> @@ -587,7 +587,8 @@ int os_find_u_boot(char *fname, int maxlen)
>  	/* Look for 'u-boot' in the parent directory of spl/ */
>  	p = strstr(fname, "/spl/");

I have built sandbox_spl_defconfig.

First observation: any parallel build fails. But that is to be handled
in a seperate patch.

The logic to find u-boot here is broken. From the top directory of the
git repository I executed:

$ spl/u-boot-spl

U-Boot SPL 2018.07-rc1-00063-g277daa5c0f (Jun 09 2018 - 20:40:23 +0200)
Trying to boot from sandbox
(spl/u-boot not found, error -2)
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

Why would you expect fname to start with /spl/?

In my case fname = 'spl/u-boot'. Why do you test for '/spl/' (only)?

Just for the record if I execute
$ ./spl/u-boot-spl
everything works fine. But who would precede a directory name with './'?

Should we not concatenate the spl executable and the u-boot executable
during the build process so that spl/u-boot knows where to find u-boot?
That would be much safer than any assumption about relative paths.

Best regards

Heinrich

>  	if (p) {
> -		strcpy(p, p + 4);
> +		/* Remove the "/spl" characters */
> +		memmove(p, p + 4, strlen(p + 4) + 1);
>  		fd = os_open(fname, O_RDONLY);
>  		if (fd >= 0) {
>  			close(fd);
>