[libvirt] [PATCH] Avoid segfault in virt-aa-helper when handling read-only mount filesystems

[Martin Kletzander]

On Tue, Aug 23, 2016 at 07:30:04PM +0100, rufo wrote:
>This patch fixes a segfault in virt-aa-helper caused by attempting to modify a string literal in situ.
>It is triggered when a domain has a <filesystem> with type='mount' configured readonly, and libvirt is using the AppArmor security driver for sVirt confinement.
>---

Thanks for the patch.  If I may, I'd suggest some teeny tiny fixes:

 - Wrap the longer lines above ^^

 - You also need to free that copied string

 - Please use real name for posting patches instead of pseudonyms (I
   would otherwise fix the previous ones and push the patch

> src/security/virt-aa-helper.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
>index 49e12b9..c22aa66 100644
>--- a/src/security/virt-aa-helper.c
>+++ b/src/security/virt-aa-helper.c
>@@ -740,6 +740,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
>     bool readonly = true;
>     bool explicit_deny_rule = true;
>     char *sub = NULL;
>+    char *perms_new = strdup(perms);
>
>     if (path == NULL)
>         return rc;
>@@ -764,12 +765,12 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
>         return rc;
>     }
>
>-    if (strchr(perms, 'w') != NULL) {
>+    if (strchr(perms_new, 'w') != NULL) {
>         readonly = false;
>         explicit_deny_rule = false;
>     }
>
>-    if ((sub = strchr(perms, 'R')) != NULL) {
>+    if ((sub = strchr(perms_new, 'R')) != NULL) {
>         /* Don't write the invalid R permission, replace it with 'r' */
>         sub[0] = 'r';
>         explicit_deny_rule = false;
>@@ -787,7 +788,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
>     if (tmp[strlen(tmp) - 1] == '/')
>         tmp[strlen(tmp) - 1] = '\0';
>
>-    virBufferAsprintf(buf, "  \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms);
>+    virBufferAsprintf(buf, "  \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms_new);
>     if (explicit_deny_rule) {
>         virBufferAddLit(buf, "  # don't audit writes to readonly files\n");
>         virBufferAsprintf(buf, "  deny \"%s%s\" w,\n", tmp, recursive ? "/**" : "");
>--
>2.9.3
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160823/94941522/attachment-0001.sig>