[libvirt] [PATCH 08/10] qemuDomainCreateDevice: Don't loop endlessly
Martin Kletzander
mkletzan at redhat.com
Tue Feb 7 10:34:02 UTC 2017
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 at 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 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/20170207/705877e2/attachment-0001.sig>
More information about the libvir-list
mailing list