<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 21, 2018 at 3:02 PM, Jamie Strandboge <span dir="ltr"><<a href="mailto:jamie@canonical.com" target="_blank">jamie@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, 2018-03-21 at 13:10 +0100, Christian Ehrhardt wrote:<br>
> Recent changes have made implementing this mandatory to hot add any<br>
> memory.<br>
> Implementing this in apparmor fixes this as well as allows hot-add of<br>
> nvdimm<br>
> tpye memory with an nvdimmPath set generating a AppArmor rule for<br>
> that<br>
> path.<br>
><br>
> Example hot adding:<br>
>   <memory model='nvdimm'><br>
>     <source><br>
>       <path>/tmp/nvdimm-test</path><br>
>     </source><br>
>     <target><br>
>       <size unit='KiB'>524288</size><br>
>       <node>0</node><br>
>     </target><br>
>   </memory><br>
> Creates now:<br>
>   "/tmp/nvdimm-test" rwk,<br>
><br>
> Fixes: <a href="https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1755153" rel="noreferrer" target="_blank">https://bugs.launchpad.net/<wbr>ubuntu/+source/libvirt/+bug/<wbr>1755153</a><br>
><br>
> Signed-off-by: Christian Ehrhardt <<a href="mailto:christian.ehrhardt@canonical.com">christian.ehrhardt@canonical.<wbr>com</a>><br>
> ---<br>
>  src/security/security_<wbr>apparmor.c | 43<br>
> ++++++++++++++++++++++++++++++<wbr>++++++++++<br>
>  1 file changed, 43 insertions(+)<br>
><br>
> diff --git a/src/security/security_<wbr>apparmor.c<br>
> b/src/security/security_<wbr>apparmor.c<br>
> index a989992..7509552 100644<br>
> --- a/src/security/security_<wbr>apparmor.c<br>
> +++ b/src/security/security_<wbr>apparmor.c<br>
> @@ -718,6 +718,46 @@<br>
> AppArmorRestoreSecurityDiskLab<wbr>el(virSecurityManagerPtr mgr,<br>
><br>
>  /* Called when hotplugging */<br>
>  static int<br>
> +AppArmorSetMemoryLabel(<wbr>virSecurityManagerPtr mgr,<br>
> +                       virDomainDefPtr def,<br>
> +                       virDomainMemoryDefPtr mem)<br>
> +{<br>
> +    switch ((virDomainMemoryModel) mem->model) {<br>
<br>
</div></div>Perhaps check if mem->model is not NULL? Sorry for not noticing this<br>
before...<br>
<span class=""><br></span></blockquote><div>yeah could be a good cautious thing to do as well.</div><div>No problem, this can easily be respun to become better.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +    case VIR_DOMAIN_MEMORY_MODEL_<wbr>NVDIMM:<br>
> +        if (!virFileExists(mem-><wbr>nvdimmPath)) {<br>
<br>
</span>and the same for mem->nvdimmPath?<br></blockquote><div><br></div><div>yep</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
> +            virReportError(VIR_ERR_<wbr>INTERNAL_ERROR,<br>
> +                           _("%s: \'%s\' does not exist"),<br>
> +                           __func__, mem->nvdimmPath);<br>
> +            return -1;<br>
> +        }<br>
> +        return reload_profile(mgr, def, mem->nvdimmPath, true);<br>
> +        break;<br>
> +    case VIR_DOMAIN_MEMORY_MODEL_NONE:<br>
> +    case VIR_DOMAIN_MEMORY_MODEL_DIMM:<br>
> +    case VIR_DOMAIN_MEMORY_MODEL_LAST:<br>
> +        break;<br>
> +    }<br>
> +<br>
> +    return 0;<br>
> +}<br>
> +<br>
> +<br>
> +static int<br>
> +AppArmorRestoreMemoryLabel(<wbr>virSecurityManagerPtr mgr,<br>
> +                           virDomainDefPtr def,<br>
> +                           virDomainMemoryDefPtr mem<br>
> ATTRIBUTE_UNUSED)<br>
> +{<br>
> +    virSecurityLabelDefPtr secdef =<br>
> +        virDomainDefGetSecurityLabelDe<wbr>f(def,<br>
> SECURITY_APPARMOR_NAME);<br>
> +<br>
> +    if (!secdef || !secdef->relabel)<br>
> +        return 0;<br>
> +<br>
<br>
</div></div>You forgot to remove the secdef tests here too (they are already in<br>
reload_profile).</blockquote><div> </div><div>I'm sure I dropped it, maybe I squashed with the wrong commit oO</div><div>Will certainly take a look at this ...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> +    return reload_profile(mgr, def, NULL, false);<br>
> +}<br>
> +<br>
> +/* Called when hotplugging */<br>
> +static int<br>
>  AppArmorSetSecurityImageLabel(<wbr>virSecurityManagerPtr mgr,<br>
>                                virDomainDefPtr def,<br>
>                                virStorageSourcePtr src)<br>
> @@ -1115,6 +1155,9 @@ virSecurityDriver virAppArmorSecurityDriver = {<br>
>      .domainSetSecurityImageLabel        =<br>
> AppArmorSetSecurityImageLabel,<br>
>      .<wbr>domainRestoreSecurityImageLabe<wbr>l    =<br>
> AppArmorRestoreSecurityImageLa<wbr>bel,<br>
><br>
> +    .domainSetSecurityMemoryLabel       = AppArmorSetMemoryLabel,<br>
> +    .<wbr>domainRestoreSecurityMemoryLab<wbr>el   =<br>
> AppArmorRestoreMemoryLabel,<br>
> +<br>
>      .<wbr>domainSetSecurityDaemonSocketL<wbr>abel =<br>
> AppArmorSetSecurityDaemonSocke<wbr>tLabel,<br>
>      .domainSetSecuritySocketLabel       =<br>
> AppArmorSetSecuritySocketLabel<wbr>,<br>
>      .<wbr>domainClearSecuritySocketLabel     =<br>
> AppArmorClearSecuritySocketLab<wbr>el,<br>
--<br>
</div></div><span class="HOEnZb"><font color="#888888">Jamie Strandboge             | <a href="http://www.canonical.com" rel="noreferrer" target="_blank">http://www.canonical.com</a></font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Christian Ehrhardt</span><div style="color:rgb(136,136,136);font-size:12.8px">Software Engineer, Ubuntu Server</div><div style="color:rgb(136,136,136);font-size:12.8px">Canonical Ltd</div></div></div></div></div>
</div></div>