[libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices

[Jamie Strandboge]

On Tue, 22 Oct 2019, Christian Ehrhardt wrote:

> Shared memory devices need qemu to be able to access certain paths
> either for the shared memory directly (mostly ivshmem-plain) or for a
> socket (mostly ivshmem-doorbell).
> 
> Add logic to virt-aa-helper to render those apparmor rules based
> on the domain configuration.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1761645
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> ---
>  src/security/virt-aa-helper.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 7d7262ca39..8c261f0010 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -958,6 +958,7 @@ get_files(vahControl * ctl)
>      int rc = -1;
>      size_t i;
>      char *uuid;
> +    char *mem_path = NULL;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      bool needsVfio = false, needsvhost = false, needsgl = false;
>  
> @@ -1224,6 +1225,39 @@ get_files(vahControl * ctl)
>          }
>      }
>  
> +    for (i = 0; i < ctl->def->nshmems; i++) {
> +        if (ctl->def->shmems[i]) {
> +            virDomainShmemDef *shmem = ctl->def->shmems[i];
> +            /* server path can be on any type and overwrites defaults */
> +            if (shmem->server.enabled &&
> +                shmem->server.chr.data.nix.path) {
> +                    if (vah_add_file(&buf, shmem->server.chr.data.nix.path,
> +                            "rw") != 0)
> +                        goto cleanup;

I'll let others comment on the code changes, but this apparmor rule
looks ok.

> +            } else {

That said, I wonder about the logic here since up above we have:

  if (shmem->server.enabled && shmem->server.chr.data.nix.path)

but here we just have 'else'. What happens if server.enabled is false
and server.chr.data.nix.path is set or vice versa? Does this 'else'
clause correctly handle those scenarios?


> +                switch (shmem->model) {
> +                case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
> +                    /* until exposed, recreate qemuBuildShmemBackendMemProps */
> +                    if (virAsprintf(&mem_path, "/dev/shm/%s", shmem->name) < 0)
> +                        goto cleanup;
> +                    break;
> +                case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
> +                case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
> +                     /* until exposed, recreate qemuDomainPrepareShmemChardev */
> +                    if (virAsprintf(&mem_path, "/var/lib/libvirt/shmem-%s-sock",
> +                            shmem->name) < 0)
> +                        goto cleanup;
> +                    break;
> +                }
> +                if (mem_path != NULL) {
> +                    if (vah_add_file(&buf, mem_path, "rw") != 0)
> +                        goto cleanup;
> +                }
> +            }
> +        }
> +    }
> +


-- 
Jamie Strandboge             | http://www.canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191119/8fbf4763/attachment-0001.sig>