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.
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list