[libvirt-users] virDomainCoreDumpWithFormat files created as root

[noxdafox]

On 18/11/15 12:11, Michal Privoznik wrote:
> On 08.11.2015 12:43, noxdafox wrote:
>> I've been spending a bit of time looking into libvirt's code and I
>> believe this is not implemented as Daniel first said.
>>
>> The issue is in the qemuOpenFileAs function in src/qemu/qemu_driver.c
>> which ignores the dynamic ownership flag and does not set correct
>> ownership on the file.
>>
>> The qemuOpenFileAs function is used by other ones as well, so I wonder
>> if this affects other QEMU features.
>>
>> I tried to fix and test it in different use cases and I didn't notice
>> any side effect, yet I'd like to provide tests as this function is quite
>> a core one and it's implementation is a bit confusing (maybe a
>> refactoring would simplify its logic).
>> Unfortunately the function is not exposed, therefore unittests are a bit
>> challenging. Higher level tests seem to mock the driver
>> (src/test/test_driver.c) as well.
>>
>> Here's the patch fixing the issue. I set the correct uid and gid only if
>> the file is being created and dynamic ownership is set.
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index a2cc002..1b47dc6 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -2932,6 +2932,11 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t
>> fallback_gid,
>>           if (path_shared <= 0 || dynamicOwnership)
>>               vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
>>
>> +        if (dynamicOwnership) {
>> +            uid = fallback_uid;
>> +            gid = fallback_gid;
>> +        }
>> +
>>           if (stat(path, &sb) == 0) {
>>               /* It already exists, we don't want to delete it on error */
>>               need_unlink = false;
>>
>> Please let me know what do you think about it and if we can somehow
>> include it in the sources. I'd be glad to offer further help if necessary.
> Can you send it as an actual patch? Looks reasonable to me.
>
> Michal
Done,

please let me know if I did something wrong as it's the first patch I'm 
sending.

BR.