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.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list