* [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL.
@ 2018-10-09 7:45 Kan Li
2018-10-12 18:58 ` Laurent Vivier
2018-10-12 19:02 ` Laurent Vivier
0 siblings, 2 replies; 4+ messages in thread
From: Kan Li @ 2018-10-09 7:45 UTC (permalink / raw)
To: qemu-devel
Summary:
This is to fix bug https://bugs.launchpad.net/qemu/+bug/1796754.
It is valid for ifc_buf to be NULL according to
http://man7.org/linux/man-pages/man7/netdevice.7.html.
Signed-off-by: Kan Li <likan_999.student@sina.com>
---
linux-user/syscall.c | 56 ++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ae3c0dfef7..fbab98d4f7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4134,28 +4134,33 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
unlock_user(argptr, arg, 0);
host_ifconf = (struct ifconf *)(unsigned long)buf_temp;
- target_ifc_len = host_ifconf->ifc_len;
target_ifc_buf = (abi_long)(unsigned long)host_ifconf->ifc_buf;
- target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
- nb_ifreq = target_ifc_len / target_ifreq_size;
- host_ifc_len = nb_ifreq * sizeof(struct ifreq);
+ if (target_ifc_buf != 0) {
+ target_ifc_len = host_ifconf->ifc_len;
- outbufsz = sizeof(*host_ifconf) + host_ifc_len;
- if (outbufsz > MAX_STRUCT_SIZE) {
- /* We can't fit all the extents into the fixed size buffer.
- * Allocate one that is large enough and use it instead.
- */
- host_ifconf = malloc(outbufsz);
- if (!host_ifconf) {
- return -TARGET_ENOMEM;
+ target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
+ nb_ifreq = target_ifc_len / target_ifreq_size;
+ host_ifc_len = nb_ifreq * sizeof(struct ifreq);
+
+ outbufsz = sizeof(*host_ifconf) + host_ifc_len;
+ if (outbufsz > MAX_STRUCT_SIZE) {
+ /* We can't fit all the extents into the fixed size buffer.
+ * Allocate one that is large enough and use it instead.
+ */
+ host_ifconf = malloc(outbufsz);
+ if (!host_ifconf) {
+ return -TARGET_ENOMEM;
+ }
+ memcpy(host_ifconf, buf_temp, sizeof(*host_ifconf));
+ free_buf = 1;
}
- memcpy(host_ifconf, buf_temp, sizeof(*host_ifconf));
- free_buf = 1;
- }
- host_ifc_buf = (char*)host_ifconf + sizeof(*host_ifconf);
+ host_ifc_buf = (char*)host_ifconf + sizeof(*host_ifconf);
- host_ifconf->ifc_len = host_ifc_len;
+ host_ifconf->ifc_len = host_ifc_len;
+ } else {
+ host_ifc_buf = NULL;
+ }
host_ifconf->ifc_buf = host_ifc_buf;
ret = get_errno(safe_ioctl(fd, ie->host_cmd, host_ifconf));
@@ -4178,15 +4183,16 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
thunk_convert(argptr, host_ifconf, arg_type, THUNK_TARGET);
unlock_user(argptr, arg, target_size);
- /* copy ifreq[] to target user */
-
- argptr = lock_user(VERIFY_WRITE, target_ifc_buf, target_ifc_len, 0);
- for (i = 0; i < nb_ifreq ; i++) {
- thunk_convert(argptr + i * target_ifreq_size,
- host_ifc_buf + i * sizeof(struct ifreq),
- ifreq_arg_type, THUNK_TARGET);
+ if (target_ifc_buf != 0) {
+ /* copy ifreq[] to target user */
+ argptr = lock_user(VERIFY_WRITE, target_ifc_buf, target_ifc_len, 0);
+ for (i = 0; i < nb_ifreq ; i++) {
+ thunk_convert(argptr + i * target_ifreq_size,
+ host_ifc_buf + i * sizeof(struct ifreq),
+ ifreq_arg_type, THUNK_TARGET);
+ }
+ unlock_user(argptr, target_ifc_buf, target_ifc_len);
}
- unlock_user(argptr, target_ifc_buf, target_ifc_len);
}
if (free_buf) {
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL.
2018-10-09 7:45 [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL Kan Li
@ 2018-10-12 18:58 ` Laurent Vivier
2018-10-12 19:02 ` Laurent Vivier
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Vivier @ 2018-10-12 18:58 UTC (permalink / raw)
To: Kan Li, qemu-devel
On 09/10/2018 09:45, Kan Li wrote:
> Summary:
> This is to fix bug https://bugs.launchpad.net/qemu/+bug/1796754.
> It is valid for ifc_buf to be NULL according to
> http://man7.org/linux/man-pages/man7/netdevice.7.html.
>
> Signed-off-by: Kan Li <likan_999.student@sina.com>
> ---
> linux-user/syscall.c | 56 ++++++++++++++++++++++++--------------------
> 1 file changed, 31 insertions(+), 25 deletions(-)
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL.
2018-10-09 7:45 [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL Kan Li
2018-10-12 18:58 ` Laurent Vivier
@ 2018-10-12 19:02 ` Laurent Vivier
2018-10-13 18:34 ` Laurent Vivier
1 sibling, 1 reply; 4+ messages in thread
From: Laurent Vivier @ 2018-10-12 19:02 UTC (permalink / raw)
To: Kan Li, qemu-devel
On 09/10/2018 09:45, Kan Li wrote:
> Summary:
> This is to fix bug https://bugs.launchpad.net/qemu/+bug/1796754.
> It is valid for ifc_buf to be NULL according to
> http://man7.org/linux/man-pages/man7/netdevice.7.html.
>
> Signed-off-by: Kan Li <likan_999.student@sina.com>
> ---
> linux-user/syscall.c | 56 ++++++++++++++++++++++++--------------------
> 1 file changed, 31 insertions(+), 25 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ae3c0dfef7..fbab98d4f7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4134,28 +4134,33 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
> unlock_user(argptr, arg, 0);
>
> host_ifconf = (struct ifconf *)(unsigned long)buf_temp;
> - target_ifc_len = host_ifconf->ifc_len;
> target_ifc_buf = (abi_long)(unsigned long)host_ifconf->ifc_buf;
>
> - target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
> - nb_ifreq = target_ifc_len / target_ifreq_size;
> - host_ifc_len = nb_ifreq * sizeof(struct ifreq);
> + if (target_ifc_buf != 0) {
> + target_ifc_len = host_ifconf->ifc_len;
>
> - outbufsz = sizeof(*host_ifconf) + host_ifc_len;
> - if (outbufsz > MAX_STRUCT_SIZE) {
> - /* We can't fit all the extents into the fixed size buffer.
> - * Allocate one that is large enough and use it instead.
> - */
> - host_ifconf = malloc(outbufsz);
> - if (!host_ifconf) {
> - return -TARGET_ENOMEM;
> + target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
> + nb_ifreq = target_ifc_len / target_ifreq_size;
> + host_ifc_len = nb_ifreq * sizeof(struct ifreq);
> +
> + outbufsz = sizeof(*host_ifconf) + host_ifc_len;
> + if (outbufsz > MAX_STRUCT_SIZE) {
> + /* We can't fit all the extents into the fixed size buffer.
> + * Allocate one that is large enough and use it instead.
> + */
> + host_ifconf = malloc(outbufsz);
> + if (!host_ifconf) {
> + return -TARGET_ENOMEM;
> + }
> + memcpy(host_ifconf, buf_temp, sizeof(*host_ifconf));
> + free_buf = 1;
> }
> - memcpy(host_ifconf, buf_temp, sizeof(*host_ifconf));
> - free_buf = 1;
> - }
> - host_ifc_buf = (char*)host_ifconf + sizeof(*host_ifconf);
> + host_ifc_buf = (char*)host_ifconf + sizeof(*host_ifconf);
>
> - host_ifconf->ifc_len = host_ifc_len;
> + host_ifconf->ifc_len = host_ifc_len;
> + } else {
> + host_ifc_buf = NULL;
> + }
> host_ifconf->ifc_buf = host_ifc_buf;
>
> ret = get_errno(safe_ioctl(fd, ie->host_cmd, host_ifconf));
> @@ -4178,15 +4183,16 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
> thunk_convert(argptr, host_ifconf, arg_type, THUNK_TARGET);
> unlock_user(argptr, arg, target_size);
>
> - /* copy ifreq[] to target user */
> -
> - argptr = lock_user(VERIFY_WRITE, target_ifc_buf, target_ifc_len, 0);
> - for (i = 0; i < nb_ifreq ; i++) {
> - thunk_convert(argptr + i * target_ifreq_size,
> - host_ifc_buf + i * sizeof(struct ifreq),
> - ifreq_arg_type, THUNK_TARGET);
> + if (target_ifc_buf != 0) {
> + /* copy ifreq[] to target user */
> + argptr = lock_user(VERIFY_WRITE, target_ifc_buf, target_ifc_len, 0);
> + for (i = 0; i < nb_ifreq ; i++) {
> + thunk_convert(argptr + i * target_ifreq_size,
> + host_ifc_buf + i * sizeof(struct ifreq),
> + ifreq_arg_type, THUNK_TARGET);
> + }
> + unlock_user(argptr, target_ifc_buf, target_ifc_len);
> }
> - unlock_user(argptr, target_ifc_buf, target_ifc_len);
> }
>
> if (free_buf) {
>
Applied to my branch linux-user-for-3.1
Thanks,
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL.
2018-10-12 19:02 ` Laurent Vivier
@ 2018-10-13 18:34 ` Laurent Vivier
0 siblings, 0 replies; 4+ messages in thread
From: Laurent Vivier @ 2018-10-13 18:34 UTC (permalink / raw)
To: Kan Li, qemu-devel
On 12/10/2018 21:02, Laurent Vivier wrote:
> On 09/10/2018 09:45, Kan Li wrote:
>> Summary:
>> This is to fix bug https://bugs.launchpad.net/qemu/+bug/1796754.
>> It is valid for ifc_buf to be NULL according to
>> http://man7.org/linux/man-pages/man7/netdevice.7.html.
>>
>> Signed-off-by: Kan Li <likan_999.student@sina.com>
>> ---
>> linux-user/syscall.c | 56 ++++++++++++++++++++++++--------------------
>> 1 file changed, 31 insertions(+), 25 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index ae3c0dfef7..fbab98d4f7 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4134,28 +4134,33 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
>> unlock_user(argptr, arg, 0);
>>
>> host_ifconf = (struct ifconf *)(unsigned long)buf_temp;
>> - target_ifc_len = host_ifconf->ifc_len;
>> target_ifc_buf = (abi_long)(unsigned long)host_ifconf->ifc_buf;
>>
>> - target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
>> - nb_ifreq = target_ifc_len / target_ifreq_size;
>> - host_ifc_len = nb_ifreq * sizeof(struct ifreq);
>> + if (target_ifc_buf != 0) {
>> + target_ifc_len = host_ifconf->ifc_len;
>>
>> - outbufsz = sizeof(*host_ifconf) + host_ifc_len;
>> - if (outbufsz > MAX_STRUCT_SIZE) {
>> - /* We can't fit all the extents into the fixed size buffer.
>> - * Allocate one that is large enough and use it instead.
>> - */
>> - host_ifconf = malloc(outbufsz);
>> - if (!host_ifconf) {
>> - return -TARGET_ENOMEM;
>> + target_ifreq_size = thunk_type_size(ifreq_arg_type, 0);
In fact, the target_ifreq_size is used later even if target_ifc_buf is
NULL, so you have to move it out of the "if" body.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-13 18:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-09 7:45 [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL Kan Li
2018-10-12 18:58 ` Laurent Vivier
2018-10-12 19:02 ` Laurent Vivier
2018-10-13 18:34 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.