Re: [Qemu-devel] [PATCH] linux-user: Fix crashes in ioctl(SIOCGIFCONF) when ifc_buf is NULL.

[Laurent Vivier]

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 <address@hidden>
>> ---
>>  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