Guido Günther wrote:
> On Fri, Feb 03, 2017 at 10:32:12AM -0700, Jim Fehlig wrote:
>> If the apparmor security driver is loaded/enabled and domain config
>> contains a <seclabel> element whose type attribute is not 'apparmor',
>> starting the domain fails when attempting to label resources such
>> as tap FDs.
>>
>> Many of the apparmor driver entry points attempt to retrieve the
>> apparmor security label from the domain def, returning failure if
>> not found. Functions such as AppArmorSetFDLabel fail even though
>> domain config contains an explicit 'none' secuirty driver, e.g.
>>
>> <seclabel type='none' model='none'/>
>>
>> Change the entry points to succeed if the domain config <seclabel>
>> is not apparmor. This matches the behavior of the selinux driver.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
>> ---
>> src/security/security_apparmor.c | 58 ++++++++++++----------------------------
>> 1 file changed, 17 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
>> index ad50b08..f871e60 100644
>> --- a/src/security/security_apparmor.c
>> +++ b/src/security/security_apparmor.c
>> @@ -289,10 +289,7 @@ reload_profile(virSecurityManagerPtr mgr,
>> virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(
>> def, SECURITY_APPARMOR_NAME);
>>
>> - if (!secdef)
>> - return rc;
>> -
>> - if (!secdef->relabel)
>> + if (!secdef || !secdef->relabel)
>> return 0;
>>
>> if ((profile_name = get_profile_name(def)) == NULL)
>> @@ -435,7 +432,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>> SECURITY_APPARMOR_NAME);
>>
>> if (!secdef)
>> - return -1;
>> + return 0;
>>
>> if ((secdef->type == VIR_DOMAIN_SECLABEL_STATIC) ||
>> (secdef->type == VIR_DOMAIN_SECLABEL_NONE))
>> @@ -495,10 +492,7 @@ AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
>> {
>> virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
>> SECURITY_APPARMOR_NAME);
>> - if (!secdef)
>> - return -1;
>> -
>> - if (!secdef->relabel)
>> + if (!secdef || !secdef->relabel)
>> return 0;
>>
>> /* Reload the profile if stdin_path is specified. Note that
>> @@ -559,12 +553,11 @@ AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>> {
>> virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
>> SECURITY_APPARMOR_NAME);
>> - if (!secdef)
>> - return -1;
>> -
>> - VIR_FREE(secdef->model);
>> - VIR_FREE(secdef->label);
>> - VIR_FREE(secdef->imagelabel);
>> + if (secdef) {
>> + VIR_FREE(secdef->model);
>> + VIR_FREE(secdef->label);
>> + VIR_FREE(secdef->imagelabel);\
>
> The trailing slash can be dropped here. The rest of th code looks good
> to me.
Opps, thanks for catching that. I've removed it in my local branch.
Do we need someone else with knowledge of the apparmor driver to review and ACK
these changes?
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
