Without this patch, virFileResolveLink could misbehave when applied to a file on a POSIX-nonconforming file system. The fix is to use gnulib's areadlink module, but it's GPL'd. I'm ready to relax it to LGPLv2+, and have asked the other author for his rubber stamp. Once that's fixed, this change will depend on having an up-to-date gnulib submodule. That will be the subject of my next message. >From a3133b2e8b1453578b30e4b9c83c7473feb7c65b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Tue, 15 Dec 2009 08:27:53 +0100 Subject: [PATCH] avoid malfunction when virFileResolveLink is applied to non-POSIX FS The virFileResolveLink utility function relied on the POSIX guarantee that stat.st_size of a symlink is the length of the value. However, on some types of file systems, it is invalid, so do not rely on it. Use gnulib's areadlink module instead. * bootstrap (modules): Add areadlink. * src/util/util.c: Include "areadlink.h". Let areadlink perform the readlink and malloc. * configure.in (AC_CHECK_FUNCS): Remove readlink. No need, since it's presence is guaranteed by gnulib. --- bootstrap | 1 + configure.in | 2 +- src/util/util.c | 27 +++------------------------ 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/bootstrap b/bootstrap index 667af4f..c07d851 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit modules=' +areadlink base64 c-ctype close diff --git a/configure.in b/configure.in index 6135932..6ed2efd 100644 --- a/configure.in +++ b/configure.in @@ -83,7 +83,7 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap readlink]) +AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) dnl Availability of various not common threadsafe functions AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) diff --git a/src/util/util.c b/src/util/util.c index 694838a..44a4b2f 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -64,6 +64,7 @@ #include <mntent.h> #endif +#include "areadlink.h" #include "virterror_internal.h" #include "logging.h" #include "event.h" @@ -1059,10 +1060,7 @@ int virFileLinkPointsTo(const char *checkLink, int virFileResolveLink(const char *linkpath, char **resultpath) { -#ifdef HAVE_READLINK struct stat st; - char *buf; - int n; *resultpath = NULL; @@ -1075,28 +1073,9 @@ int virFileResolveLink(const char *linkpath, return 0; } - /* Posix says that 'st_size' field from - * result of an lstat() call is filled with - * number of bytes in the destination - * filename. - */ - if (VIR_ALLOC_N(buf, st.st_size + 1) < 0) - return -ENOMEM; - - if ((n = readlink(linkpath, buf, st.st_size)) < 0) { - VIR_FREE(buf); - return -errno; - } - - buf[n] = '\0'; + *resultpath = areadlink (linkpath); - *resultpath = buf; - return 0; -#else - if (!(*resultpath = strdup(linkpath))) - return -ENOMEM; - return 0; -#endif + return *resultpath == NULL ? -1 : 0; } /*

Without this patch, virFileResolveLink could misbehave when applied to a file on a POSIX-nonconforming file system. The fix is to use gnulib's areadlink module, but it's GPL'd. I'm ready to relax it to LGPLv2+, and have asked the other author for his rubber stamp. Once that's fixed, this change will depend on having an up-to-date gnulib submodule. That will be the subject of my next message. >From a3133b2e8b1453578b30e4b9c83c7473feb7c65b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Tue, 15 Dec 2009 08:27:53 +0100 Subject: [PATCH] avoid malfunction when virFileResolveLink is applied to non-POSIX FS The virFileResolveLink utility function relied on the POSIX guarantee that stat.st_size of a symlink is the length of the value. However, on some types of file systems, it is invalid, so do not rely on it. Use gnulib's areadlink module instead. * bootstrap (modules): Add areadlink. * src/util/util.c: Include "areadlink.h". Let areadlink perform the readlink and malloc. * configure.in (AC_CHECK_FUNCS): Remove readlink. No need, since it's presence is guaranteed by gnulib. --- bootstrap | 1 + configure.in | 2 +- src/util/util.c | 27 +++------------------------ 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/bootstrap b/bootstrap index 667af4f..c07d851 100755 --- a/bootstrap +++ b/bootstrap @@ -65,6 +65,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit modules=' +areadlink base64 c-ctype close diff --git a/configure.in b/configure.in index 6135932..6ed2efd 100644 --- a/configure.in +++ b/configure.in @@ -83,7 +83,7 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap readlink]) +AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) dnl Availability of various not common threadsafe functions AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) diff --git a/src/util/util.c b/src/util/util.c index 694838a..44a4b2f 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -64,6 +64,7 @@ #include <mntent.h> #endif +#include "areadlink.h" #include "virterror_internal.h" #include "logging.h" #include "event.h" @@ -1059,10 +1060,7 @@ int virFileLinkPointsTo(const char *checkLink, int virFileResolveLink(const char *linkpath, char **resultpath) { -#ifdef HAVE_READLINK struct stat st; - char *buf; - int n; *resultpath = NULL; @@ -1075,28 +1073,9 @@ int virFileResolveLink(const char *linkpath, return 0; } - /* Posix says that 'st_size' field from - * result of an lstat() call is filled with - * number of bytes in the destination - * filename. - */ - if (VIR_ALLOC_N(buf, st.st_size + 1) < 0) - return -ENOMEM; - - if ((n = readlink(linkpath, buf, st.st_size)) < 0) { - VIR_FREE(buf); - return -errno; - } - - buf[n] = '\0'; + *resultpath = areadlink (linkpath); - *resultpath = buf; - return 0; -#else - if (!(*resultpath = strdup(linkpath))) - return -ENOMEM; - return 0; -#endif + return *resultpath == NULL ? -1 : 0; } /*