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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux