[libvirt] [PATCH] lock qemu_driver early in qemuGetSchedulerParametersFlags()

[Wen Congyang]

At 06/28/2011 10:50 PM, Eric Blake Write:
> On 06/28/2011 04:09 AM, Michal Privoznik wrote:
>> On 28.06.2011 09:58, Wen Congyang wrote:
>>> If we pass VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG to
>>> qemuGetSchedulerParametersFlags() or *nparams is less than 1,
>>> we will unlock qemu_driver without locking it. It's very dangerous.
>>>
>>> We should lock qemu_driver after calling virCheckFlags().
>>>
>>> ---
>>>  src/qemu/qemu_driver.c |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index 768e0f2..c6994cd 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -5832,6 +5832,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom,
>>>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>>>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
>>>  
>>> +    qemuDriverLock(driver);
>>> +
>>>      if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) ==
>>>          (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) {
>>>          qemuReportError(VIR_ERR_INVALID_ARG, "%s",
>>> @@ -5845,7 +5847,6 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom,
>>>          goto cleanup;
>>>      }
> 
> An alternative would have been to replace the 'goto cleanup' with
> 'return -1'.  In fact, with that alternative, we can error out without
> even spending time grabbing the lock.  Then again, the error is
> unexpected (you can assume that most callers are compliant), and
> optimizing for the rare bad code won't have any impact to the execution
> speed of good code, so I'm happy with the patch as-is.

Yes, it is another way to fix this problem, and the error is unexpected.
I do not modify it and pushed it.

Thanks.

>