static code analyzer complain:
linux-user/syscall.c:5575:9: warning: Dereference of undefined pointer value
if (*host_rt_dev_ptr != 0) {
^~~~~~~~~~~~~~~~
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
linux-user/syscall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index cec8428589..c541e22693 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5526,7 +5526,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
int target_size;
void *argptr;
abi_ulong *target_rt_dev_ptr;
- unsigned long *host_rt_dev_ptr;
+ unsigned long *host_rt_dev_ptr = NULL;
abi_long ret;
int i;
@@ -5572,7 +5572,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
unlock_user(argptr, arg, 0);
ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
- if (*host_rt_dev_ptr != 0) {
+ if (host_rt_dev_ptr != NULL && *host_rt_dev_ptr != 0) {
unlock_user((void *)*host_rt_dev_ptr,
*target_rt_dev_ptr, 0);
}
--
2.11.0
On 06/05/2017 02:41, Philippe Mathieu-Daudé wrote: > static code analyzer complain: > > linux-user/syscall.c:5575:9: warning: Dereference of undefined pointer value > if (*host_rt_dev_ptr != 0) { > ^~~~~~~~~~~~~~~~ > > Reported-by: Clang Static Analyzer > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > linux-user/syscall.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index cec8428589..c541e22693 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -5526,7 +5526,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, > int target_size; > void *argptr; > abi_ulong *target_rt_dev_ptr; > - unsigned long *host_rt_dev_ptr; > + unsigned long *host_rt_dev_ptr = NULL; > abi_long ret; > int i; > > @@ -5572,7 +5572,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, > unlock_user(argptr, arg, 0); > > ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp)); I think we should "assert(host_rt_dev_ptr)" here. It's a bug if host_rt_dev_ptr is not set. The "for" loop scans the structure to find the rt_dev field, and we should always enter in the first "if", so "host_rt_dev_ptr" is always set. > - if (*host_rt_dev_ptr != 0) { > + if (host_rt_dev_ptr != NULL && *host_rt_dev_ptr != 0) { > unlock_user((void *)*host_rt_dev_ptr, > *target_rt_dev_ptr, 0); > } > Laurent
On 05/10/2017 09:32 AM, Laurent Vivier wrote: > On 06/05/2017 02:41, Philippe Mathieu-Daudé wrote: >> static code analyzer complain: >> >> linux-user/syscall.c:5575:9: warning: Dereference of undefined pointer value >> if (*host_rt_dev_ptr != 0) { >> ^~~~~~~~~~~~~~~~ >> >> Reported-by: Clang Static Analyzer >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> linux-user/syscall.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index cec8428589..c541e22693 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -5526,7 +5526,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, >> int target_size; >> void *argptr; >> abi_ulong *target_rt_dev_ptr; >> - unsigned long *host_rt_dev_ptr; >> + unsigned long *host_rt_dev_ptr = NULL; >> abi_long ret; >> int i; >> >> @@ -5572,7 +5572,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, >> unlock_user(argptr, arg, 0); >> >> ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp)); > > I think we should "assert(host_rt_dev_ptr)" here. It's a bug if > host_rt_dev_ptr is not set. > > The "for" loop scans the structure to find the rt_dev field, and we > should always enter in the first "if", so "host_rt_dev_ptr" is always set. It's better indeed, I'll use g_assert_nonnull() and resend. Thanks! > >> - if (*host_rt_dev_ptr != 0) { >> + if (host_rt_dev_ptr != NULL && *host_rt_dev_ptr != 0) { >> unlock_user((void *)*host_rt_dev_ptr, >> *target_rt_dev_ptr, 0); >> } >> > > Laurent >
>> >> The "for" loop scans the structure to find the rt_dev field, and we >> should always enter in the first "if", so "host_rt_dev_ptr" is always >> set. > > It's better indeed, I'll use g_assert_nonnull() and resend. Careful - we haven't yet resolved whether g_assert_nonnull() is safe to use. https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05499.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.