| Message ID | 1426345935-7721-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Under Review |
| Headers | show |
On 14 March 2015 at 15:12, Stefan Weil <sw@weilnetz.de> wrote: > This fixes a warning from Coverity: > "Dereference null return value (NULL_RETURNS)" > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > linux-user/flatload.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/linux-user/flatload.c b/linux-user/flatload.c > index 566a7a8..56ac790 100644 > --- a/linux-user/flatload.c > +++ b/linux-user/flatload.c > @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len, > abi_ulong offset) > { > void *buf; > - int ret; > + int ret = -TARGET_EFAULT; The return value for this function should be a host errno, not a target errno, I think. (If you track back upwards then eventually main.c calls loader_exec() and treats the return value as a host errno.) This seems like the right thing given that these loader functions are all pre-run setup, and are not involved in emulation of guest syscalls. > > buf = lock_user(VERIFY_WRITE, ptr, len, 0); > - ret = pread(fd, buf, len, offset); > - unlock_user(buf, ptr, len); > + if (buf) { > + ret = pread(fd, buf, len, offset); A different bug, but if ret here indicates that pread() failed we should be returning -errno. > + unlock_user(buf, ptr, len); > + } > return ret; > } -- PMM
On 14/03/2015 16:12, Stefan Weil wrote: > This fixes a warning from Coverity: > "Dereference null return value (NULL_RETURNS)" > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > linux-user/flatload.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/linux-user/flatload.c b/linux-user/flatload.c > index 566a7a8..56ac790 100644 > --- a/linux-user/flatload.c > +++ b/linux-user/flatload.c > @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len, > abi_ulong offset) > { > void *buf; > - int ret; > + int ret = -TARGET_EFAULT; > > buf = lock_user(VERIFY_WRITE, ptr, len, 0); > - ret = pread(fd, buf, len, offset); > - unlock_user(buf, ptr, len); > + if (buf) { > + ret = pread(fd, buf, len, offset); > + unlock_user(buf, ptr, len); > + } > return ret; > } > /****************************************************************************/ > Hi Stefan, are you going to update and resend this patch? Thanks, Paolo
diff --git a/linux-user/flatload.c b/linux-user/flatload.c index 566a7a8..56ac790 100644 --- a/linux-user/flatload.c +++ b/linux-user/flatload.c @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len, abi_ulong offset) { void *buf; - int ret; + int ret = -TARGET_EFAULT; buf = lock_user(VERIFY_WRITE, ptr, len, 0); - ret = pread(fd, buf, len, offset); - unlock_user(buf, ptr, len); + if (buf) { + ret = pread(fd, buf, len, offset); + unlock_user(buf, ptr, len); + } return ret; } /****************************************************************************/
This fixes a warning from Coverity: "Dereference null return value (NULL_RETURNS)" Signed-off-by: Stefan Weil <sw@weilnetz.de> --- linux-user/flatload.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)