Re: [PATCH v3 3/3] qga-win: prevent crash when executing guest-file-read with large count

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 3/24/20 2:20 PM, Philippe Mathieu-Daudé wrote:
> On 3/11/20 6:04 PM, Basil Salman wrote:
>> BZ: #1594054
> 
> ^ This is not very helpful as it... (think to ppl with no knowledge of 
> 'BZ', what to do with this number). Instead ...
> 
>> guest-file-read command is currently implemented to read from a
>> file handle count number of bytes. when executed with a very large 
>> count number
>> qemu-ga crashes.
>> after some digging turns out that qemu-ga crashes after trying to 
>> allocate
>> a buffer large enough to save the data read in it, the buffer was 
>> allocated using
>> g_malloc0 which is not fail safe, and results a crash in case of failure.
>> g_malloc0 was replaced with g_try_malloc0() which returns NULL on 
>> failure,
>> A check was added for that case in order to prevent qemu-ga from crashing
>> and to send a response to the qemu-ga client accordingly.
>>
> 
> ... add here (see 
> https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message): 
> 
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1594054

And per the BZ info, please also credit the reporter:

Reported-by: Fakhri Zulkifli <mohdfakhrizulkifli@gmail.com>

> 
> Also add:
> 
> Cc: qemu-stable@nongnu.org
> 
>> Signed-off-by: Basil Salman <basil@daynix.com>
>> ---
>>   qga/commands-win32.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 9c744d6405..b49920e201 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -343,7 +343,13 @@ GuestFileRead *qmp_guest_file_read(int64_t 
>> handle, bool has_count,
>>       }
>>       fh = gfh->fh;
>> -    buf = g_malloc0(count+1);
>> +    buf = g_try_malloc0(count + 1);
>> +    if (!buf) {
>> +        error_setg(errp,
>> +                   "failed to allocate sufficient memory "
>> +                   "to complete the requested service");
>> +        return NULL;
>> +    }
> 
> Can you fix the equivalent problem in qga/commands-posix.c too please?
> 
> Also use "PATCH-for-5.0" in the patch subject so we don't miss it for 
> the next release.
> 
> Thanks!
> 
> Phil.
> 
>>       is_ok = ReadFile(fh, buf, count, &read_count, NULL);
>>       if (!is_ok) {
>>           error_setg_win32(errp, GetLastError(), "failed to read file");
>>