| Message ID | 20181113225521.49103-1-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,v3] sandbox: Use memmove() to move overlapping regions | expand |
On 11/13/2018 11:55 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 memmove() instead. > > Reported-by: Coverity (CID: 173279) > Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Alexander Graf <agraf@suse.de> Alex > --- > > Changes in v3: > - Fix commit message to say memmove() instead of memcpy() > > Changes in v2: > - Also remove the leading / from the "/spl" path > - Correct the string calculation > > arch/sandbox/cpu/os.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c > index 07e46471fe5..04669bfc177 100644 > --- a/arch/sandbox/cpu/os.c > +++ b/arch/sandbox/cpu/os.c > @@ -641,9 +641,10 @@ int os_find_u_boot(char *fname, int maxlen) > } > > /* Look for 'u-boot' in the parent directory of spl/ */ > - p = strstr(fname, "/spl/"); > + p = strstr(fname, "spl/"); > 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);
On 11/13/2018 11:55 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 memmove() instead. > > Reported-by: Coverity (CID: 173279) > Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Alexander Graf <agraf@suse.de> Alex > --- > > Changes in v3: > - Fix commit message to say memmove() instead of memcpy() > > Changes in v2: > - Also remove the leading / from the "/spl" path > - Correct the string calculation > > arch/sandbox/cpu/os.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Applied to u-boot-dm/master, thanks!
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 07e46471fe5..04669bfc177 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -641,9 +641,10 @@ int os_find_u_boot(char *fname, int maxlen) } /* Look for 'u-boot' in the parent directory of spl/ */ - p = strstr(fname, "/spl/"); + p = strstr(fname, "spl/"); 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);
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 memmove() instead. Reported-by: Coverity (CID: 173279) Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v3: - Fix commit message to say memmove() instead of memcpy() Changes in v2: - Also remove the leading / from the "/spl" path - Correct the string calculation arch/sandbox/cpu/os.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)