On Wed, Mar 21, 2018 at 3:02 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Wed, 2018-03-21 at 13:10 +0100, Christian Ehrhardt wrote:
Recent changes have made implementing this mandatory to hot add any memory. Implementing this in apparmor fixes this as well as allows hot-add of nvdimm tpye memory with an nvdimmPath set generating a AppArmor rule for that path.
Example hot adding: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-test</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Creates now: "/tmp/nvdimm-test" rwk,
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1755153
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index a989992..7509552 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -718,6 +718,46 @@ AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr,
/* Called when hotplugging */ static int +AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + switch ((virDomainMemoryModel) mem->model) {
Perhaps check if mem->model is not NULL? Sorry for not noticing this before...
yeah could be a good cautious thing to do as well. No problem, this can easily be respun to become better.
+ case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (!virFileExists(mem->nvdimmPath)) {
and the same for mem->nvdimmPath?
yep
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, mem->nvdimmPath); + return -1; + } + return reload_profile(mgr, def, mem->nvdimmPath, true); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; +
You forgot to remove the secdef tests here too (they are already in reload_profile).
I'm sure I dropped it, maybe I squashed with the wrong commit oO Will certainly take a look at this ...
+ return reload_profile(mgr, def, NULL, false); +} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1115,6 +1155,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel,
+ .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, + .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel, -- Jamie Strandboge | http://www.canonical.com
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd