[libvirt] [PATCH] Fix crash in SELinuxSecurityVerify
Laine Stump
laine at laine.org
Thu Jan 13 15:00:49 UTC 2011
On 01/13/2011 08:46 AM, Daniel P. Berrange wrote:
> On Thu, Jan 13, 2011 at 12:30:30AM -0500, Laine Stump wrote:
>> When attempting to edit a domain, libvirtd segfaulted in
>> SELinuxSecurityVerify() on this line:
>>
>> if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
>>
>> because secdef->model was NULL. Although I'm too tired to investigate
>> in depth, I noticed that all the other functions in that file that do
>> the same STREQ() will first check that def->seclabel.label is
>> non-NULL, but this function doesn't. I also noticed that label *is*
>> NULL in my case, so I tried adding that check to
>> SELinuxSecurityVerify(), and the crash goes away.
>>
>> I have no idea if this is the correct fix, but it allowed me to
>> continue my testing of a new (unrelated) feature.
>> ---
>> src/security/security_selinux.c | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
>> index d06afde..b97ca4c 100644
>> --- a/src/security/security_selinux.c
>> +++ b/src/security/security_selinux.c
>> @@ -871,6 +871,10 @@ SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>> virDomainDefPtr def)
>> {
>> const virSecurityLabelDefPtr secdef =&def->seclabel;
>> +
>> + if (def->seclabel.label == NULL)
>> + return 0;
>> +
>> if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {
>> virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
>> _("security label driver mismatch: "
> We don't want to skip a NULL label, but rather a NULL model.
Yeah, it didn't really make sense at the time, but all the other
functions were doing it, it stopped the crash, and I was too tired to
spend brain cells understanding the code :-)
> So I think you actually need to add a check
>
> if (def->seclabel.model == NULL)
> return 0;
>
> but in the Verify method in src/security/security_manager.c so
> that all drivers are protected instead of just SELinux.
Okay. Is that needed for the other methods that end up comparing
secdef->model, too? Or are you guaranteed a non-null model by the time
you get into any of those? (eg virSecurityManagerSetProcessLabel(),
virSecurityManagerSetSecuritySocketLabel(),etc)
More information about the libvir-list
mailing list