[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] openvzGetProcessInfo: address clang-detected low-probability flaw



Eric Blake wrote:
> On 04/14/2010 02:46 AM, Jim Meyering wrote:
>> From: Jim Meyering <meyering redhat com>
>>
>> * src/openvz/openvz_driver.c (openvzGetProcessInfo): Reorganize
>> so that unexpected /proc/vz/vestat content cannot make us use
>> uninitialized variables.  Without this change, an input line with
>> a matching "readvps", but fewer than 4 numbers would result in our
>> using at least "systime" uninitialized.
>> ---
>>  src/openvz/openvz_driver.c |   30 +++++++++++++++---------------
>>  1 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
>> index 95c4236..47004d6 100644
>> --- a/src/openvz/openvz_driver.c
>> +++ b/src/openvz/openvz_driver.c
>> @@ -1384,14 +1384,15 @@ static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) {
>>      int fd;
>>      char line[1024] ;
>>      unsigned long long usertime, systime, nicetime;
>> -    int readvps = 0, ret;
>> +    int readvps = vpsid + 1;  /* ensure readvps is initially different */
>> +    int ret;
>>
>> -        if (sscanf(line, "%d %llu %llu %llu",
>> -                          &readvps, &usertime, &nicetime, &systime) != 4)
>> -            continue;
>> -
>> -        if (readvps == vpsid)
>> -            break; /*found vpsid*/
>> +        if (sscanf (line, "%d %llu %llu %llu",
>> +                    &readvps, &usertime, &nicetime, &systime) == 4
>> +            && readvps == vpsid) { /*found vpsid*/
>> +            /* convert jiffies to nanoseconds */
>> +            *cpuTime = (1000ull * 1000ull * 1000ull
>> +                        * (usertime + nicetime  + systime)
>> +                        / (unsigned long long)sysconf(_SC_CLK_TCK));
>> +            break;
>> +        }
>
> ACK that the rewrite fixes the problem.  However, there's still the

Thanks for the review.

> issue that we're using sscanf in the first place, instead of
> virStrToLong_ull; do you want to prepare a followup patch, or shall I?

I am in no big hurry on that front, at least not for this
particular case, where the risk of bogus input is probably negligible.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]