[libvirt] [PATCH 0/3] apparmor: bug and typo fix and add tapFD relabeling
These three patches are generated when trying to make use of libvirt apparmor security driver in openSUSU 11.4. Patches fix some typoes and bugs and rename AppArmorSetImageFDLabel to AppArmorSetFDLabel as the common labeling function that could be used by domainSetSecurityImageFDLabel and domainSetSecurityTapFDLabel. tapfd could be seen in the last line in specific profile named /etc/apparmor.d/libvirt/libvirt-cdbebdfa-1d6d-65c3-be0f-fd74b978a773.files # DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT. "/var/log/libvirt/**/rhel6qcow2.log" w, "/var/lib/libvirt/**/rhel6qcow2.monitor" rw, "/var/run/libvirt/**/rhel6qcow2.pid" rwk, "/run/libvirt/**/rhel6qcow2.pid" rwk, "/var/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2" rw, "/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2" rw, "/var/lib/libvirt/images/rhel6u3qcow2.img" rw, "/dev/tap45" rw, Guannan Ren(3) [PATCH 1/3] apparmor: fix typoes in virt-aa-helper [PATCH 2/3] apparmor: no need to check security model [PATCH 3/3] apparmor: use AppArmorSetFDLabel for both imageFD and tapFD src/security/security_apparmor.c | 22 ++++++---------------- src/security/virt-aa-helper.c | 7 +++---- 2 files changed, 9 insertions(+), 20 deletions(-)
--- src/security/virt-aa-helper.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 263fc92..3c3449c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -924,10 +924,9 @@ get_files(vahControl * ctl) /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ - disk->chain = virStorageFileGetMetadata(disk->src, disk->format, - -1, -1, - ctl->allowDiskFormatProbing, - NULL); + disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format, + -1, -1, + ctl->allowDiskFormatProbing); /* XXX passing ignoreOpenFailure = true to get back to the behavior * from before using virDomainDiskDefForeachPath. actually we should -- 1.7.3.4
On Thu, Oct 25, 2012 at 14:51:37 +0800, Guannan Ren wrote:
--- src/security/virt-aa-helper.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 263fc92..3c3449c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -924,10 +924,9 @@ get_files(vahControl * ctl) /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ - disk->chain = virStorageFileGetMetadata(disk->src, disk->format, - -1, -1, - ctl->allowDiskFormatProbing, - NULL); + disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format, + -1, -1, + ctl->allowDiskFormatProbing);
/* XXX passing ignoreOpenFailure = true to get back to the behavior * from before using virDomainDiskDefForeachPath. actually we should
This could potentially lead to leaking the memory pointed to be previous value of disk->backingChain. Anyway, just drop this patch completely since I already sent the fix for this issue yesterday and pushed it now as commit 0111b409a393bd4c3f138419bc9bf5cb20ebbc77 Author: Jiri Denemark <jdenemar@redhat.com> Date: Wed Oct 24 12:13:48 2012 +0200 Fix build with apparmor Recent storage patches changed signature of virStorageFileGetMetadata and replaced chain with backingChain in virDomainDiskDef. Jirka
The security model has been set already when allocating and adding virSecurityLabelDefPtr struct into virDomainDefPtr, so the value of secdef->model is always non-null. So error: security label already defined for VM is reported. The patch is to remove the checking of its value in If condition clause --- src/security/security_apparmor.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 1315fe1..1972ab0 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -435,8 +435,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return rc; } - if ((secdef->label) || - (secdef->model) || (secdef->imagelabel)) { + if ((secdef->label) || (secdef->imagelabel)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); @@ -460,8 +459,8 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto err; } - secdef->model = strdup(SECURITY_APPARMOR_NAME); - if (!secdef->model) { + if (!secdef->model && + !(secdef->model = strdup(SECURITY_APPARMOR_NAME))) { virReportOOMError(); goto err; } -- 1.7.3.4
Rename AppArmorSetImageFDLabel to AppArmorSetFDLabel which could be used as a common function for *ALL* fd relabelling in Linux. In apparmor profile for specific vm with uuid cdbebdfa-1d6d-65c3-be0f-fd74b978a773 Path: /etc/apparmor.d/libvirt/libvirt-cdbebdfa-1d6d-65c3-be0f-fd74b978a773.files The last line is for the tapfd relabelling. # DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT. "/var/log/libvirt/**/rhel6qcow2.log" w, "/var/lib/libvirt/**/rhel6qcow2.monitor" rw, "/var/run/libvirt/**/rhel6qcow2.pid" rwk, "/run/libvirt/**/rhel6qcow2.pid" rwk, "/var/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2" rw, "/run/libvirt/**/*.tunnelmigrate.dest.rhel6qcow2" rw, "/var/lib/libvirt/images/rhel6u3qcow2.img" rw, "/dev/tap45" rw, --- src/security/security_apparmor.c | 15 +++------------ 1 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 1972ab0..953775c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -840,7 +840,7 @@ AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, } static int -AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, +AppArmorSetFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, int fd) { @@ -871,15 +871,6 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, fd_path, true); } -/* TODO need code here */ -static int -AppArmorSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED) -{ - return 0; -} - virSecurityDriver virAppArmorSecurityDriver = { .privateDataLen = 0, .name = SECURITY_APPARMOR_NAME, @@ -915,6 +906,6 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel, - .domainSetSecurityImageFDLabel = AppArmorSetImageFDLabel, - .domainSetSecurityTapFDLabel = AppArmorSetTapFDLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, + .domainSetSecurityTapFDLabel = AppArmorSetFDLabel, }; -- 1.7.3.4
participants (2)
-
Guannan Ren -
Jiri Denemark