7:18 a.m.
Cole was recently adding a few of the usual apparmor suspects to BZ 1761645
and I was taking a look at the low hanging fruits of it today. It isn't
perfect, but would resolve the reported issue - so I'd appreciate a
review.
Limitations:
- One could break the path creating elements in qemuBuildShmemBackendMemProps
and qemuDomainPrepareShmemChardev into extra functions and then use those
from virt-aa-helper. But I haven't done so yet and unless it is strictly
required consider it too much for what we want/need to achieve here.
- I haven't covered hotplug of shmem devices yet, it seems there is no
infrastructure for their labels yet and I wasn't sure how important
shmem-hotplug would even be to anyone.
Christian Ehrhardt (2):
virt-aa-helper: add rules for shmem devices
virt-aa-helper: testcase for shmem devices
src/security/virt-aa-helper.c | 35 +++++++++++++++++++++++++++++++++++
tests/virt-aa-helper-test | 15 +++++++++++++++
2 files changed, 50 insertions(+)
--
2.23.0
7:18 a.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
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(a)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;
+ } else {
+ 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;
+ }
+ }
+ }
+ }
+
+
if (ctl->def->tpm) {
char *shortName = NULL;
const char *tpmpath = NULL;
@@ -1324,6 +1358,7 @@ get_files(vahControl * ctl)
ctl->files = virBufferContentAndReset(&buf);
cleanup:
+ VIR_FREE(mem_path);
VIR_FREE(uuid);
return rc;
}
--
2.23.0
6:15 p.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
On 10/22/19 8:18 AM, 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(a)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]) {
shmems[i] should never be NULL here except in the case of a serious and
obvious bug elsewhere in libvirt. So this should be removed IMO. Same
goes for all other device list handling in this function, but that's a
separate issue.
Also style comment, IMO in cases where this type of pattern _is_
relevant, it's nicer to do
if (!condition)
continue;
Which saves a lot of indenting
+ 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;
+ } else {
+ 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;
virAsprintf is gone in git, you can use g_strdup_printf, which also
means dropping the error checking. See the other conversions patches
With those changes, for the series:
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
+ 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;
+ }
+ }
+ }
+ }
+
+
if (ctl->def->tpm) {
char *shortName = NULL;
const char *tpmpath = NULL;
@@ -1324,6 +1358,7 @@ get_files(vahControl * ctl)
ctl->files = virBufferContentAndReset(&buf);
cleanup:
+ VIR_FREE(mem_path);
VIR_FREE(uuid);
return rc;
}
- Cole
5:15 a.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
On Thu, Nov 14, 2019 at 1:14 AM Cole Robinson <crobinso(a)redhat.com> wrote:
On 10/22/19 8:18 AM, 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(a)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]) {
shmems[i] should never be NULL here except in the case of a serious and
obvious bug elsewhere in libvirt. So this should be removed IMO. Same
goes for all other device list handling in this function, but that's a
separate issue.
Hi Cole and thanks for the review!
I continued the pattern which existed for ages, but I agree and thank
you for pointing this one out!
I have reworked it and will append a patch that generally removes
those as I agree these days this should be safe since this is no interim
structure but a freshly parsed definition.
OTOH you could say that is defensive programming, we will see if
there is different feedback on the explicit patch about it.
Also style comment, IMO in cases where this type of pattern _is_
relevant, it's nicer to do
if (!condition)
continue;
Which saves a lot of indenting
> + 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;
> + } else {
> + 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;
virAsprintf is gone in git, you can use g_strdup_printf, which also
means dropping the error checking. See the other conversions patches
Yeah I knew this was coming, but wanted to wait for general reviews to
not proliferate this series with v1..v99 :-)
Done in v2
With those changes, for the series:
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
> + 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;
> + }
> + }
> + }
> + }
> +
> +
> if (ctl->def->tpm) {
> char *shortName = NULL;
> const char *tpmpath = NULL;
> @@ -1324,6 +1358,7 @@ get_files(vahControl * ctl)
> ctl->files = virBufferContentAndReset(&buf);
>
> cleanup:
> + VIR_FREE(mem_path);
> VIR_FREE(uuid);
> return rc;
> }
>
- Cole
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
3:25 p.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
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(a)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
8:44 a.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
On Tue, Nov 19, 2019 at 10:25 PM Jamie Strandboge <jamie(a)canonical.com> wrote:
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(a)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?
Yes if either of the above isn't fulfilled it will fall back to use
the default paths.
So a single else without other checks should be correct.
The following switch then differs the default paths used base on the model.
> + 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
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
11:22 a.m.
New subject: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices
On Wed, 20 Nov 2019, Christian Ehrhardt wrote:
On Tue, Nov 19, 2019 at 10:25 PM Jamie Strandboge
<jamie(a)canonical.com> wrote:
> On Tue, 22 Oct 2019, Christian Ehrhardt wrote:
> > + 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?
Yes if either of the above isn't fulfilled it will fall back to use
the default paths.
So a single else without other checks should be correct.
The following switch then differs the default paths used base on the model.
Thanks! We agreed on irc a small code comment would clarify this. LGTM
provided you add a code comment similar to what we discussed on IRC.
--
Jamie Strandboge | http://www.canonical.com
7:18 a.m.
New subject: [libvirt] [PATCH 2/2] virt-aa-helper: testcase for shmem devices
Adding build time self tests for basic (deprecated), doorbell and plain mode.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/virt-aa-helper-test | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 6e674bfe5c..6a6703ecf5 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -384,6 +384,21 @@ testme "0" "dri egl" "-r -u
$valid_uuid" "$test_xml" "/dev/dri/testegl1.*rw,$"
sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<graphics type='spice'><gl enable='yes'
rendernode='/dev/dri/testegl2'/></graphics></devices>,g"
"$template_xml" > "$test_xml"
testme "0" "dri spice" "-r -u $valid_uuid"
"$test_xml" "/dev/dri/testegl2.*rw,$"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem'/><size
unit='M'>4</size></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem" "-r -u $valid_uuid"
"$test_xml"
"\"/var/lib/libvirt/shmem-my_shmem0-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem'/><size unit='M'>4</size><server
path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem serverpath" "-r -u $valid_uuid"
"$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem-plain'/><size
unit='M'>4</size></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem plain" "-r -u $valid_uuid"
"$test_xml" "\"/dev/shm/my_shmem0\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='shmem_server'><model
type='ivshmem-doorbell'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem doorbell" "-r -u $valid_uuid"
"$test_xml"
"\"/var/lib/libvirt/shmem-shmem_server-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='shmem_server'><model
type='ivshmem-doorbell'/><server
path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem doorbell serverpath" "-r -u $valid_uuid"
"$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
testme "0" "help" "-h"
echo "" >$output
--
2.23.0
5:20 a.m.
New subject: [libvirt] [PATCH v2 0/3] virt-aa-helper for shmem device
Cole was recently adding a few of the usual apparmor suspects to BZ 1761645
and I was taking a look at the low hanging fruits of it today. It isn't
perfect, but would resolve the reported issue - so I'd appreciate a
review.
Limitations:
- One could break the path creating elements in qemuBuildShmemBackendMemProps
and qemuDomainPrepareShmemChardev into extra functions and then use those
from virt-aa-helper. But I haven't done so yet and unless it is strictly
required consider it too much for what we want/need to achieve here.
- I haven't covered hotplug of shmem devices yet, it seems there is no
infrastructure for their labels yet and I wasn't sure how important
shmem-hotplug would even be to anyone.
Updates in v2:
- rebase latest master
- drop checking shmems[i]
- switch to use g_strdup_printf
- added an extra patch removing checks like ctl->def->mems[i]
- add reviewed-by tag
Christian Ehrhardt (3):
virt-aa-helper: add rules for shmem devices
virt-aa-helper: testcase for shmem devices
virt-aa-helper: drop pointer checks in get_files
src/security/virt-aa-helper.c | 166 +++++++++++++++++++---------------
tests/virt-aa-helper-test | 15 +++
2 files changed, 109 insertions(+), 72 deletions(-)
--
2.24.0
5:20 a.m.
New subject: [libvirt] [PATCH v2 1/3] virt-aa-helper: add rules for shmem devices
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
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/security/virt-aa-helper.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5ac9a9eeb8..c6c4bb9bd0 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -929,6 +929,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;
@@ -1192,6 +1193,35 @@ get_files(vahControl * ctl)
}
}
+ for (i = 0; i < ctl->def->nshmems; 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;
+ } else {
+ switch (shmem->model) {
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+ /* until exposed, recreate qemuBuildShmemBackendMemProps */
+ mem_path = g_strdup_printf("/dev/shm/%s", shmem->name);
+ break;
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
+ case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
+ /* until exposed, recreate qemuDomainPrepareShmemChardev */
+ mem_path = g_strdup_printf("/var/lib/libvirt/shmem-%s-sock",
+ shmem->name);
+ break;
+ }
+ if (mem_path != NULL) {
+ if (vah_add_file(&buf, mem_path, "rw") != 0)
+ goto cleanup;
+ }
+ }
+ }
+
+
if (ctl->def->tpm) {
char *shortName = NULL;
const char *tpmpath = NULL;
@@ -1286,6 +1316,7 @@ get_files(vahControl * ctl)
ctl->files = virBufferContentAndReset(&buf);
cleanup:
+ VIR_FREE(mem_path);
VIR_FREE(uuid);
return rc;
}
--
2.24.0
5:20 a.m.
New subject: [libvirt] [PATCH v2 2/3] virt-aa-helper: testcase for shmem devices
Adding build time self tests for basic (deprecated), doorbell and plain mode.
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/virt-aa-helper-test | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 6e674bfe5c..6a6703ecf5 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -384,6 +384,21 @@ testme "0" "dri egl" "-r -u
$valid_uuid" "$test_xml" "/dev/dri/testegl1.*rw,$"
sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<graphics type='spice'><gl enable='yes'
rendernode='/dev/dri/testegl2'/></graphics></devices>,g"
"$template_xml" > "$test_xml"
testme "0" "dri spice" "-r -u $valid_uuid"
"$test_xml" "/dev/dri/testegl2.*rw,$"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem'/><size
unit='M'>4</size></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem" "-r -u $valid_uuid"
"$test_xml"
"\"/var/lib/libvirt/shmem-my_shmem0-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem'/><size unit='M'>4</size><server
path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem serverpath" "-r -u $valid_uuid"
"$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='my_shmem0'><model
type='ivshmem-plain'/><size
unit='M'>4</size></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem plain" "-r -u $valid_uuid"
"$test_xml" "\"/dev/shm/my_shmem0\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='shmem_server'><model
type='ivshmem-doorbell'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem doorbell" "-r -u $valid_uuid"
"$test_xml"
"\"/var/lib/libvirt/shmem-shmem_server-sock\"\s*rw,$"
+
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e
"s,</devices>,<shmem name='shmem_server'><model
type='ivshmem-doorbell'/><server
path='/var/lib/libvirt/ivshmem_socket'/></shmem></devices>,g"
"$template_xml" > "$test_xml"
+testme "0" "shmem doorbell serverpath" "-r -u $valid_uuid"
"$test_xml" "\"/var/lib/libvirt/ivshmem_socket\"\s*rw,$"
+
testme "0" "help" "-h"
echo "" >$output
--
2.24.0
5:20 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
It was mentioned that the pointers in loops like:
for (i = 0; i < ctl->def->nserials; i++)
if (ctl->def->serials[i] ...
will always be !NULL or we would have a much more serious problem.
Simplify the if chains in get_files by dropping these checks.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------
1 file changed, 63 insertions(+), 72 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c6c4bb9bd0..17f49a6259 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -965,8 +965,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nserials; i++)
- if (ctl->def->serials[i] &&
- (ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -980,8 +979,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nconsoles; i++)
- if (ctl->def->consoles[i] &&
- (ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -993,8 +991,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nparallels; i++)
- if (ctl->def->parallels[i] &&
- (ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -1008,8 +1005,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nchannels; i++)
- if (ctl->def->channels[i] &&
- (ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -1082,76 +1078,74 @@ get_files(vahControl * ctl)
"r") != 0)
goto cleanup;
- for (i = 0; i < ctl->def->nhostdevs; i++)
- if (ctl->def->hostdevs[i]) {
- virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
- virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
- switch (dev->source.subsys.type) {
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- virUSBDevicePtr usb =
- virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
+ for (i = 0; i < ctl->def->nhostdevs; i++) {
+ virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
+ virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
+ switch (dev->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
+ virUSBDevicePtr usb =
+ virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
- if (usb == NULL)
- continue;
-
- if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
- continue;
+ if (usb == NULL)
+ continue;
- rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
- virUSBDeviceFree(usb);
- if (rc != 0)
- goto cleanup;
- break;
- }
+ if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
+ continue;
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
- virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
- switch ((virMediatedDeviceModelType) mdevsrc->model) {
- case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
- case VIR_MDEV_MODEL_TYPE_VFIO_AP:
- case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
- needsVfio = true;
- break;
- case VIR_MDEV_MODEL_TYPE_LAST:
- default:
- virReportEnumRangeError(virMediatedDeviceModelType,
- mdevsrc->model);
- break;
- }
- break;
- }
-
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- virPCIDevicePtr pci = virPCIDeviceNew(
- dev->source.subsys.u.pci.addr.domain,
- dev->source.subsys.u.pci.addr.bus,
- dev->source.subsys.u.pci.addr.slot,
- dev->source.subsys.u.pci.addr.function);
+ rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
+ virUSBDeviceFree(usb);
+ if (rc != 0)
+ goto cleanup;
+ break;
+ }
- virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
- if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
- backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+ virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
+ switch ((virMediatedDeviceModelType) mdevsrc->model) {
+ case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+ case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+ case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
needsVfio = true;
- }
+ break;
+ case VIR_MDEV_MODEL_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virMediatedDeviceModelType,
+ mdevsrc->model);
+ break;
+ }
+ break;
+ }
- if (pci == NULL)
- continue;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
+ virPCIDevicePtr pci = virPCIDeviceNew(
+ dev->source.subsys.u.pci.addr.domain,
+ dev->source.subsys.u.pci.addr.bus,
+ dev->source.subsys.u.pci.addr.slot,
+ dev->source.subsys.u.pci.addr.function);
+
+ virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
+ if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
+ backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ needsVfio = true;
+ }
- rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
- virPCIDeviceFree(pci);
+ if (pci == NULL)
+ continue;
- break;
- }
+ rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
+ virPCIDeviceFree(pci);
- default:
- rc = 0;
- break;
- } /* switch */
+ break;
}
+ default:
+ rc = 0;
+ break;
+ } /* switch */
+ }
+
for (i = 0; i < ctl->def->nfss; i++) {
- if (ctl->def->fss[i] &&
- ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
+ if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
(ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH
||
ctl->def->fss[i]->fsdriver ==
VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) &&
ctl->def->fss[i]->src) {
@@ -1166,16 +1160,14 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->ninputs; i++) {
- if (ctl->def->inputs[i] &&
- ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)
{
+ if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev,
"rw") != 0)
goto cleanup;
}
}
for (i = 0; i < ctl->def->nnets; i++) {
- if (ctl->def->nets[i] &&
- ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
+ if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
ctl->def->nets[i]->data.vhostuser) {
virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;
@@ -1186,8 +1178,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nmems; i++) {
- if (ctl->def->mems[i] &&
- ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath,
"rw") != 0)
goto cleanup;
}
--
2.24.0
9:11 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/14/19 6:20 AM, Christian Ehrhardt wrote:
It was mentioned that the pointers in loops like:
for (i = 0; i < ctl->def->nserials; i++)
if (ctl->def->serials[i] ...
will always be !NULL or we would have a much more serious problem.
Simplify the if chains in get_files by dropping these checks.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------
1 file changed, 63 insertions(+), 72 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
3:31 p.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
It was mentioned that the pointers in loops like:
for (i = 0; i < ctl->def->nserials; i++)
if (ctl->def->serials[i] ...
will always be !NULL or we would have a much more serious problem.
Simplify the if chains in get_files by dropping these checks.
I don't really see why a NULL check is a problem so this feels a bit
like busy work and it seems short-circuiting and not doing is exactly
what you would want to do in the event of a 'much more serious problem'.
Is this really required? +0 to apply on principle (I've not reviewed the
changes closely).
Signed-off-by: Christian Ehrhardt
<christian.ehrhardt(a)canonical.com>
---
src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------
1 file changed, 63 insertions(+), 72 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c6c4bb9bd0..17f49a6259 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -965,8 +965,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nserials; i++)
- if (ctl->def->serials[i] &&
- (ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->serials[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -980,8 +979,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nconsoles; i++)
- if (ctl->def->consoles[i] &&
- (ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->consoles[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -993,8 +991,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nparallels; i++)
- if (ctl->def->parallels[i] &&
- (ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->parallels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_FILE ||
ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_UNIX ||
@@ -1008,8 +1005,7 @@ get_files(vahControl * ctl)
goto cleanup;
for (i = 0; i < ctl->def->nchannels; i++)
- if (ctl->def->channels[i] &&
- (ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
+ if ((ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_PTY
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_DEV
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_FILE
||
ctl->def->channels[i]->source->type == VIR_DOMAIN_CHR_TYPE_UNIX
||
@@ -1082,76 +1078,74 @@ get_files(vahControl * ctl)
"r") != 0)
goto cleanup;
- for (i = 0; i < ctl->def->nhostdevs; i++)
- if (ctl->def->hostdevs[i]) {
- virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
- virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
- switch (dev->source.subsys.type) {
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- virUSBDevicePtr usb =
- virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
+ for (i = 0; i < ctl->def->nhostdevs; i++) {
+ virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
+ virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
+ switch (dev->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
+ virUSBDevicePtr usb =
+ virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
- if (usb == NULL)
- continue;
-
- if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
- continue;
+ if (usb == NULL)
+ continue;
- rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
- virUSBDeviceFree(usb);
- if (rc != 0)
- goto cleanup;
- break;
- }
+ if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
+ continue;
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
- virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
- switch ((virMediatedDeviceModelType) mdevsrc->model) {
- case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
- case VIR_MDEV_MODEL_TYPE_VFIO_AP:
- case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
- needsVfio = true;
- break;
- case VIR_MDEV_MODEL_TYPE_LAST:
- default:
- virReportEnumRangeError(virMediatedDeviceModelType,
- mdevsrc->model);
- break;
- }
- break;
- }
-
- case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- virPCIDevicePtr pci = virPCIDeviceNew(
- dev->source.subsys.u.pci.addr.domain,
- dev->source.subsys.u.pci.addr.bus,
- dev->source.subsys.u.pci.addr.slot,
- dev->source.subsys.u.pci.addr.function);
+ rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
+ virUSBDeviceFree(usb);
+ if (rc != 0)
+ goto cleanup;
+ break;
+ }
- virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
- if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
- backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
+ virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
+ switch ((virMediatedDeviceModelType) mdevsrc->model) {
+ case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+ case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+ case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
needsVfio = true;
- }
+ break;
+ case VIR_MDEV_MODEL_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virMediatedDeviceModelType,
+ mdevsrc->model);
+ break;
+ }
+ break;
+ }
- if (pci == NULL)
- continue;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
+ virPCIDevicePtr pci = virPCIDeviceNew(
+ dev->source.subsys.u.pci.addr.domain,
+ dev->source.subsys.u.pci.addr.bus,
+ dev->source.subsys.u.pci.addr.slot,
+ dev->source.subsys.u.pci.addr.function);
+
+ virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
+ if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
+ backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+ needsVfio = true;
+ }
- rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
- virPCIDeviceFree(pci);
+ if (pci == NULL)
+ continue;
- break;
- }
+ rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
+ virPCIDeviceFree(pci);
- default:
- rc = 0;
- break;
- } /* switch */
+ break;
}
+ default:
+ rc = 0;
+ break;
+ } /* switch */
+ }
+
for (i = 0; i < ctl->def->nfss; i++) {
- if (ctl->def->fss[i] &&
- ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
+ if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
(ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH
||
ctl->def->fss[i]->fsdriver ==
VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) &&
ctl->def->fss[i]->src) {
@@ -1166,16 +1160,14 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->ninputs; i++) {
- if (ctl->def->inputs[i] &&
- ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)
{
+ if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev,
"rw") != 0)
goto cleanup;
}
}
for (i = 0; i < ctl->def->nnets; i++) {
- if (ctl->def->nets[i] &&
- ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
+ if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
ctl->def->nets[i]->data.vhostuser) {
virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;
@@ -1186,8 +1178,7 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nmems; i++) {
- if (ctl->def->mems[i] &&
- ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+ if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath,
"rw") != 0)
goto cleanup;
}
--
2.24.0
--
Jamie Strandboge | http://www.canonical.com
8:52 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On Tue, Nov 19, 2019 at 10:31 PM Jamie Strandboge <jamie(a)canonical.com> wrote:
On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
> It was mentioned that the pointers in loops like:
> for (i = 0; i < ctl->def->nserials; i++)
> if (ctl->def->serials[i] ...
> will always be !NULL or we would have a much more serious problem.
>
> Simplify the if chains in get_files by dropping these checks.
I don't really see why a NULL check is a problem so this feels a bit
like busy work and it seems short-circuiting and not doing is exactly
what you would want to do in the event of a 'much more serious problem'.
Is this really required? +0 to apply on principle (I've not reviewed the
changes closely).
Well Cole brought it up and I intentionally wanted to have the
discussion separate to the shmem changes.
As I said in the mail on patch 2/2 in v1 of this I consider it
"defensive programming" and not strictly required to be removed.
I'm happy and fine to keep the checks as-is since they are not on a
performance critical path.
I agree as you say "in the event of a 'much more serious problem'"
this would be exactly what we want.
I'll withdraw this patch from the series then unless somebody really
shows a reason to why we should drop the checks.
> Signed-off-by: Christian Ehrhardt
<christian.ehrhardt(a)canonical.com>
> ---
> src/security/virt-aa-helper.c | 135 ++++++++++++++++------------------
> 1 file changed, 63 insertions(+), 72 deletions(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index c6c4bb9bd0..17f49a6259 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -965,8 +965,7 @@ get_files(vahControl * ctl)
> }
>
> for (i = 0; i < ctl->def->nserials; i++)
> - if (ctl->def->serials[i] &&
> - (ctl->def->serials[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> + if ((ctl->def->serials[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> ctl->def->serials[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_DEV ||
> ctl->def->serials[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_FILE ||
> ctl->def->serials[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_UNIX ||
> @@ -980,8 +979,7 @@ get_files(vahControl * ctl)
> goto cleanup;
>
> for (i = 0; i < ctl->def->nconsoles; i++)
> - if (ctl->def->consoles[i] &&
> - (ctl->def->consoles[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> + if ((ctl->def->consoles[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> ctl->def->consoles[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_DEV ||
> ctl->def->consoles[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_FILE ||
> ctl->def->consoles[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_UNIX ||
> @@ -993,8 +991,7 @@ get_files(vahControl * ctl)
> goto cleanup;
>
> for (i = 0; i < ctl->def->nparallels; i++)
> - if (ctl->def->parallels[i] &&
> - (ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> + if ((ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_DEV ||
> ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_FILE ||
> ctl->def->parallels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_UNIX ||
> @@ -1008,8 +1005,7 @@ get_files(vahControl * ctl)
> goto cleanup;
>
> for (i = 0; i < ctl->def->nchannels; i++)
> - if (ctl->def->channels[i] &&
> - (ctl->def->channels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> + if ((ctl->def->channels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY ||
> ctl->def->channels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_DEV ||
> ctl->def->channels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_FILE ||
> ctl->def->channels[i]->source->type ==
VIR_DOMAIN_CHR_TYPE_UNIX ||
> @@ -1082,76 +1078,74 @@ get_files(vahControl * ctl)
> "r") != 0)
> goto cleanup;
>
> - for (i = 0; i < ctl->def->nhostdevs; i++)
> - if (ctl->def->hostdevs[i]) {
> - virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
> - virDomainHostdevSubsysUSBPtr usbsrc =
&dev->source.subsys.u.usb;
> - switch (dev->source.subsys.type) {
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> - virUSBDevicePtr usb =
> - virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
> + for (i = 0; i < ctl->def->nhostdevs; i++) {
> + virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
> + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
> + switch (dev->source.subsys.type) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> + virUSBDevicePtr usb =
> + virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL);
>
> - if (usb == NULL)
> - continue;
> -
> - if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
> - continue;
> + if (usb == NULL)
> + continue;
>
> - rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb,
&buf);
> - virUSBDeviceFree(usb);
> - if (rc != 0)
> - goto cleanup;
> - break;
> - }
> + if (virHostdevFindUSBDevice(dev, true, &usb) < 0)
> + continue;
>
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> - virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
> - switch ((virMediatedDeviceModelType) mdevsrc->model) {
> - case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
> - case VIR_MDEV_MODEL_TYPE_VFIO_AP:
> - case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
> - needsVfio = true;
> - break;
> - case VIR_MDEV_MODEL_TYPE_LAST:
> - default:
> - virReportEnumRangeError(virMediatedDeviceModelType,
> - mdevsrc->model);
> - break;
> - }
> - break;
> - }
> -
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> - virPCIDevicePtr pci = virPCIDeviceNew(
> - dev->source.subsys.u.pci.addr.domain,
> - dev->source.subsys.u.pci.addr.bus,
> - dev->source.subsys.u.pci.addr.slot,
> - dev->source.subsys.u.pci.addr.function);
> + rc = virUSBDeviceFileIterate(usb, file_iterate_hostdev_cb, &buf);
> + virUSBDeviceFree(usb);
> + if (rc != 0)
> + goto cleanup;
> + break;
> + }
>
> - virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
> - if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
> - backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> + virDomainHostdevSubsysMediatedDevPtr mdevsrc =
&dev->source.subsys.u.mdev;
> + switch ((virMediatedDeviceModelType) mdevsrc->model) {
> + case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
> + case VIR_MDEV_MODEL_TYPE_VFIO_AP:
> + case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
> needsVfio = true;
> - }
> + break;
> + case VIR_MDEV_MODEL_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virMediatedDeviceModelType,
> + mdevsrc->model);
> + break;
> + }
> + break;
> + }
>
> - if (pci == NULL)
> - continue;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> + virPCIDevicePtr pci = virPCIDeviceNew(
> + dev->source.subsys.u.pci.addr.domain,
> + dev->source.subsys.u.pci.addr.bus,
> + dev->source.subsys.u.pci.addr.slot,
> + dev->source.subsys.u.pci.addr.function);
> +
> + virDomainHostdevSubsysPCIBackendType backend =
dev->source.subsys.u.pci.backend;
> + if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
> + backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
> + needsVfio = true;
> + }
>
> - rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
> - virPCIDeviceFree(pci);
> + if (pci == NULL)
> + continue;
>
> - break;
> - }
> + rc = virPCIDeviceFileIterate(pci, file_iterate_pci_cb, &buf);
> + virPCIDeviceFree(pci);
>
> - default:
> - rc = 0;
> - break;
> - } /* switch */
> + break;
> }
>
> + default:
> + rc = 0;
> + break;
> + } /* switch */
> + }
> +
> for (i = 0; i < ctl->def->nfss; i++) {
> - if (ctl->def->fss[i] &&
> - ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT
&&
> + if (ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT &&
> (ctl->def->fss[i]->fsdriver ==
VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
> ctl->def->fss[i]->fsdriver ==
VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) &&
> ctl->def->fss[i]->src) {
> @@ -1166,16 +1160,14 @@ get_files(vahControl * ctl)
> }
>
> for (i = 0; i < ctl->def->ninputs; i++) {
> - if (ctl->def->inputs[i] &&
> - ctl->def->inputs[i]->type ==
VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
> + if (ctl->def->inputs[i]->type ==
VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
> if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev,
"rw") != 0)
> goto cleanup;
> }
> }
>
> for (i = 0; i < ctl->def->nnets; i++) {
> - if (ctl->def->nets[i] &&
> - ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
> + if (ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
> ctl->def->nets[i]->data.vhostuser) {
> virDomainChrSourceDefPtr vhu =
ctl->def->nets[i]->data.vhostuser;
>
> @@ -1186,8 +1178,7 @@ get_files(vahControl * ctl)
> }
>
> for (i = 0; i < ctl->def->nmems; i++) {
> - if (ctl->def->mems[i] &&
> - ctl->def->mems[i]->model ==
VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> + if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath,
"rw") != 0)
> goto cleanup;
> }
> --
> 2.24.0
>
--
Jamie Strandboge | http://www.canonical.com
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
10:13 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/19/19 4:31 PM, Jamie Strandboge wrote:
On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
> It was mentioned that the pointers in loops like:
> for (i = 0; i < ctl->def->nserials; i++)
> if (ctl->def->serials[i] ...
> will always be !NULL or we would have a much more serious problem.
>
> Simplify the if chains in get_files by dropping these checks.
I don't really see why a NULL check is a problem so this feels a bit
like busy work and it seems short-circuiting and not doing is exactly
what you would want to do in the event of a 'much more serious problem'.
Is this really required? +0 to apply on principle (I've not reviewed the
changes closely).
If for example def->nserials and def->serials[i] disagree, then there is
a serious bug somewhere. The internal API contract is that it should
never happen, so code shouldn't check for the condition. I'm pretty sure
if the XML parser is creating that situation, the qemu driver would
crash in a dozen different places.
So the patch isn't strictly required, but it is an improvement IMO: it
reduces code, improves readability, and helps simplify review for other
libvirt devs who may be confused by this uncommon idiom (I was). But I
will leave it up to you guys to decide whether to push or not
Thanks,
Cole
11:25 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On Wed, 20 Nov 2019, Cole Robinson wrote:
On 11/19/19 4:31 PM, Jamie Strandboge wrote:
> On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
>
>> It was mentioned that the pointers in loops like:
>> for (i = 0; i < ctl->def->nserials; i++)
>> if (ctl->def->serials[i] ...
>> will always be !NULL or we would have a much more serious problem.
>>
>> Simplify the if chains in get_files by dropping these checks.
>
> I don't really see why a NULL check is a problem so this feels a bit
> like busy work and it seems short-circuiting and not doing is exactly
> what you would want to do in the event of a 'much more serious problem'.
> Is this really required? +0 to apply on principle (I've not reviewed the
> changes closely).
>
If for example def->nserials and def->serials[i] disagree, then there is
a serious bug somewhere. The internal API contract is that it should
never happen, so code shouldn't check for the condition. I'm pretty sure
if the XML parser is creating that situation, the qemu driver would
crash in a dozen different places.
So the patch isn't strictly required, but it is an improvement IMO: it
reduces code, improves readability, and helps simplify review for other
libvirt devs who may be confused by this uncommon idiom (I was). But I
will leave it up to you guys to decide whether to push or not
To be clear, I am intentionally not blocking on this. If, as upstream,
you'd prefer this to be a certain way for reasons that outweigh my POV,
by all means feel free to do that. The changes are mechanical and IMHO
don't need an apparmor-focused review (though if you'd prefer I go
through the full patch, I can).
--
Jamie Strandboge | http://www.canonical.com
11:53 a.m.
New subject: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/20/19 12:25 PM, Jamie Strandboge wrote:
On Wed, 20 Nov 2019, Cole Robinson wrote:
> On 11/19/19 4:31 PM, Jamie Strandboge wrote:
>> On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
>>
>>> It was mentioned that the pointers in loops like:
>>> for (i = 0; i < ctl->def->nserials; i++)
>>> if (ctl->def->serials[i] ...
>>> will always be !NULL or we would have a much more serious problem.
>>>
>>> Simplify the if chains in get_files by dropping these checks.
>>
>> I don't really see why a NULL check is a problem so this feels a bit
>> like busy work and it seems short-circuiting and not doing is exactly
>> what you would want to do in the event of a 'much more serious problem'.
>> Is this really required? +0 to apply on principle (I've not reviewed the
>> changes closely).
>>
>
> If for example def->nserials and def->serials[i] disagree, then there is
> a serious bug somewhere. The internal API contract is that it should
> never happen, so code shouldn't check for the condition. I'm pretty sure
> if the XML parser is creating that situation, the qemu driver would
> crash in a dozen different places.
>
> So the patch isn't strictly required, but it is an improvement IMO: it
> reduces code, improves readability, and helps simplify review for other
> libvirt devs who may be confused by this uncommon idiom (I was). But I
> will leave it up to you guys to decide whether to push or not
To be clear, I am intentionally not blocking on this. If, as upstream,
you'd prefer this to be a certain way for reasons that outweigh my POV,
by all means feel free to do that. The changes are mechanical and IMHO
don't need an apparmor-focused review (though if you'd prefer I go
through the full patch, I can).
This patch moves the code to be more consistent with the rest of the
libvirt code base, so I think it's good to push. I thought Christian was
waiting on you for review before pushing apparmor patches though, I
didn't want to step on any toes :)
Thanks,
Cole
1:38 a.m.
New subject: [libvirt] [PATCH v2 0/3] virt-aa-helper for shmem device
On Thu, Nov 14, 2019 at 12:20 PM Christian Ehrhardt
<christian.ehrhardt(a)canonical.com> wrote:
Cole was recently adding a few of the usual apparmor suspects to BZ 1761645
and I was taking a look at the low hanging fruits of it today. It isn't
perfect, but would resolve the reported issue - so I'd appreciate a
review.
Limitations:
- One could break the path creating elements in qemuBuildShmemBackendMemProps
and qemuDomainPrepareShmemChardev into extra functions and then use those
from virt-aa-helper. But I haven't done so yet and unless it is strictly
required consider it too much for what we want/need to achieve here.
- I haven't covered hotplug of shmem devices yet, it seems there is no
infrastructure for their labels yet and I wasn't sure how important
shmem-hotplug would even be to anyone.
Updates in v2:
- rebase latest master
- drop checking shmems[i]
- switch to use g_strdup_printf
- added an extra patch removing checks like ctl->def->mems[i]
- add reviewed-by tag
Thanks everyone for the further review and discussion.
Pushing with Ack/Review Tags after a rebuild/recheck
Christian Ehrhardt (3):
virt-aa-helper: add rules for shmem devices
virt-aa-helper: testcase for shmem devices
virt-aa-helper: drop pointer checks in get_files
src/security/virt-aa-helper.c | 166 +++++++++++++++++++---------------
tests/virt-aa-helper-test | 15 +++
2 files changed, 109 insertions(+), 72 deletions(-)
--
2.24.0
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
1828
days inactive
1858
days old
18 comments
3 participants
participants (3)
-
Christian Ehrhardt -
Cole Robinson -
Jamie Strandboge