Re: infinite loop in xenstat_qmp.c

["Reiser, Hans" <hr@xxxxxxxxxxxxxxxxx>]

Dear Andrew,
Thanks for picking this up. Sure, I agree with your proposal.
Cheers, Hans

> On 2021-01-22, at 14:25, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> 
> On 09/11/2020 14:36, Reiser, Hans wrote:
>> Hi,
>> 
>> I have seen several occasions with "dead" xentop processes consuming 100% 
>> CPU time, and tracked this down
>> to the following problem:
>> 
>> When the QEMU process the qmp_read function is communicating with 
>> terminates, qmp_read may enter an
>> infinite loop:  poll signals EOF (POLLIN and POLLHUP set), the subsequent 
>> read() call returns 0, and then the
>> function calls poll again, which still sees the EOF condition and will 
>> return again immediately with POLLIN and
>> POLLHUP set, repeating ad infinitum.
>> 
>> A simple fix is to terminate the loop when read returns 0 (under "normal" 
>> instances, poll will return with POLLIN
>> set only if there is data to read, so read will always read >0 bytes, except 
>> if the socket has been closed).
>> 
>> Cheers, Hans
> 
> Hi - this appears to have slipped through the cracks.
> 
> Thanks for the bugfix, but we require code submissions to conform to the
> DCO[1] and have a Signed-off-by line.
> 
> If you're happy to agree to that, I can fix up the patch and get it
> sorted.  We've moved this library in the time since you submitted the
> bugfix.
> 
> Thanks, and sorry for the delay.
> 
> ~Andrew
> 
> [1]
> https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Signed-off-by
> 
>> 
>> diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c 
>> b/tools/xenstat/libxenstat/src/xenstat_qmp.c
>> index 19b236e7b6..0c5748ba68 100644
>> --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
>> +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
>> @@ -298,7 +298,7 @@ static int qmp_read(int qfd, unsigned char **qstats)
>>        pfd[0].events = POLLIN;
>>        while ((n = poll(pfd, 1, 10)) > 0) {
>>                if (pfd[0].revents & POLLIN) {
>> -                       if ((n = read(qfd, buf, sizeof(buf))) < 0) {
>> +                       if ((n = read(qfd, buf, sizeof(buf))) <= 0) {
>>                                free(*qstats);
>>                                return 0;
>>                        }
>> 
>> 
>