[libvirt] [PATCH 2/3] qemuDomainAttachDeviceMknodHelper: unlink() not so often

[Martin Kletzander]

On Wed, Jan 04, 2017 at 03:13:56PM +0100, Michal Privoznik wrote:
>Not that I'd encounter any bug here, but the code doesn't look
>100% correct. Imagine, somebody is trying to attach a device to a
>domain, and the device's /dev entry already exists in the qemu
>namespace. This is handled gracefully and the control continues
>with setting up ACLs and calling security manager to set up
>labels. Now, if any of these steps fail, control jump on the
>'cleanup' label and unlink() the file straight away. Even when it
>was not us who created the file in the first place. This can be
>possibly dangerous.
>

"Don't unlink non-existing files" or something similar would be enough,
I guess :)

>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/qemu/qemu_domain.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index d05ebcb416..40bed1b396 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -7521,6 +7521,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
> {
>     struct qemuDomainAttachDeviceMknodData *data = opaque;
>     int ret = -1;
>+    bool delDevice = false;
>
>     virSecurityManagerPostFork(data->driver->securityManager);
>
>@@ -7543,6 +7544,8 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
>                                  data->file);
>             goto cleanup;
>         }
>+    } else {
>+        delDevice = true;
>     }
>
>     if (virFileSetACLs(data->file, data->acl) < 0 &&
>@@ -7606,7 +7609,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
>
>     ret = 0;
>  cleanup:
>-    if (ret < 0)
>+    if (ret < 0 && delDevice)
>         unlink(data->file);
>     virFileFreeACLs(&data->acl);
>     return ret;
>--
>2.11.0
>
>--
>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: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170104/64343837/attachment-0001.sig>