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

Re: [libvirt] [PATCH 08/10] qemuDomainCreateDevice: Don't loop endlessly



On Fri, Jan 20, 2017 at 10:42:48AM +0100, Michal Privoznik wrote:
When working with symlinks it is fairly easy to get into a loop.
Don't.

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8cbfb2d16..448583313 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6954,9 +6954,10 @@ qemuDomainGetPreservedMounts(virQEMUDriverPtr driver,


static int
-qemuDomainCreateDevice(const char *device,
-                       const char *path,
-                       bool allow_noent)
+qemuDomainCreateDeviceRecursive(const char *device,
+                                const char *path,
+                                bool allow_noent,
+                                unsigned int ttl)
{
    char *devicePath = NULL;
    char *target = NULL;
@@ -6968,6 +6969,13 @@ qemuDomainCreateDevice(const char *device,
    char *tcon = NULL;
#endif

+    if (!ttl) {
+        virReportSystemError(ELOOP,
+                             _("Too many levels of symbolic links: %s"),
+                             device);
+        return ret;
+    }
+
    if (lstat(device, &sb) < 0) {
        if (errno == ENOENT && allow_noent) {
            /* Ignore non-existent device. */
@@ -7057,7 +7065,8 @@ qemuDomainCreateDevice(const char *device,
            tmp = NULL;
        }

-        if (qemuDomainCreateDevice(target, path, allow_noent) < 0)
+        if (qemuDomainCreateDeviceRecursive(target, path,
+                                            allow_noent, ttl - 1) < 0)
            goto cleanup;
    } else {
        if (create &&
@@ -7128,6 +7137,17 @@ qemuDomainCreateDevice(const char *device,
}


+static int
+qemuDomainCreateDevice(const char *device,
+                       const char *path,
+                       bool allow_noent)
+{
+    long symloop_max = sysconf(_SC_SYMLOOP_MAX);
+
+    return qemuDomainCreateDeviceRecursive(device, path,
+                                           allow_noent, symloop_max);

So you are taking 'long' that can, officially, be '-1' and pass it to a
function as unsigned int.  Having a maximum is nice, I have no idea why
on my system there is no limit apparently (sysconf() returns -1 with no
errno set), but if the system doesn't report any (like my case above) it
effectively makes the SYMLOOP_MAX = UINT_MAX in this codepath.  Should
we avoid that or just let it be?  This way it's still better than
unlimited, so ACK to this version, but I just wanted a (short)
discussion about this.

+}
+

static int
qemuDomainPopulateDevices(virQEMUDriverPtr driver,
--
2.11.0

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

Attachment: signature.asc
Description: Digital signature


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