From 550d1e1b2d5130a95bf78bb224dad8ae8108a53e Mon Sep 17 00:00:00 2001

Message-Id: <550d1e1b2d5130a95bf78bb224dad8ae8108a53e@dist-git>

From: Eric Blake <eblake@redhat.com>

Date: Tue, 18 Feb 2014 15:45:30 -0700

Subject: [PATCH] storage: avoid short reads while chasing backing chain

https://bugzilla.redhat.com/show_bug.cgi?id=1045643

prereq of CVE-2013-6456

Our backing file chain code was not very robust to an ill-timed

EINTR, which could lead to a short read causing us to randomly

treat metadata differently than usual.  But the existing

virFileReadLimFD forces an error if we don't read the entire

file, even though we only care about the header of the file.

So add a new virFile function that does what we want.

* src/util/virfile.h (virFileReadHeaderFD): New prototype.

* src/util/virfile.c (virFileReadHeaderFD): New function.

* src/libvirt_private.syms (virfile.h): Export it.

* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)

(virStorageFileProbeFormatFromFD): Use it.

Signed-off-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 5327fad4f292e4f3f84884ffe158c492bf00519c)

Conflicts:

	src/util/virstoragefile.c: buffer signedness

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/libvirt_private.syms  |  1 +

 src/util/virfile.c        | 21 +++++++++++++++++++++

 src/util/virfile.h        |  9 ++++++---

 src/util/virstoragefile.c | 10 ++--------

 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index cda024e..852affa 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -1401,6 +1401,7 @@ virFileOpenAs;

 virFileOpenTty;

 virFilePrintf;

 virFileReadAll;

+virFileReadHeaderFD;

 virFileReadLimFD;

 virFileResolveAllLinks;

 virFileResolveLink;

diff --git a/src/util/virfile.c b/src/util/virfile.c

index 9b3a4ad..0d4a6be 100644

--- a/src/util/virfile.c

+++ b/src/util/virfile.c

@@ -1153,6 +1153,27 @@ saferead_lim(int fd, size_t max_len, size_t *length)

     return NULL;

 }

+

+/* A wrapper around saferead_lim that merely stops reading at the

+ * specified maximum size.  */

+int

+virFileReadHeaderFD(int fd, int maxlen, char **buf)

+{

+    size_t len;

+    char *s;

+

+    if (maxlen <= 0) {

+        errno = EINVAL;

+        return -1;

+    }

+    s = saferead_lim(fd, maxlen, &len;;

+    if (s == NULL)

+        return -1;

+    *buf = s;

+    return len;

+}

+

+

 /* A wrapper around saferead_lim that maps a failure due to

    exceeding the maximum size limitation to EOVERFLOW.  */

 int

diff --git a/src/util/virfile.h b/src/util/virfile.h

index f6e087e..0d20cdb 100644

--- a/src/util/virfile.h

+++ b/src/util/virfile.h

@@ -122,9 +122,12 @@ int virFileNBDDeviceAssociate(const char *file,

 int virFileDeleteTree(const char *dir);

-int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;

-

-int virFileReadAll(const char *path, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;

+int virFileReadHeaderFD(int fd, int maxlen, char **buf)

+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);

+int virFileReadLimFD(int fd, int maxlen, char **buf)

+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);

+int virFileReadAll(const char *path, int maxlen, char **buf)

+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);

 int virFileWriteStr(const char *path, const char *str, mode_t mode)

     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c

index 9453599..fc8aa90 100644

--- a/src/util/virstoragefile.c

+++ b/src/util/virstoragefile.c

@@ -788,10 +788,7 @@ virStorageFileGetMetadataInternal(const char *path,

         goto cleanup;

     }

-    if (VIR_ALLOC_N(buf, len) < 0)

-        goto cleanup;

-

-    if ((len = read(fd, buf, len)) < 0) {

+    if ((len = virFileReadHeaderFD(fd, len, (char **)&buf)) < 0) {

         virReportSystemError(errno, _("cannot read header '%s'"), path);

         goto cleanup;

     }

@@ -934,15 +931,12 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)

         return VIR_STORAGE_FILE_DIR;

     }

-    if (VIR_ALLOC_N(head, len) < 0)

-        return -1;

-

     if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {

         virReportSystemError(errno, _("cannot set to start of '%s'"), path);

         goto cleanup;

     }

-    if ((len = read(fd, head, len)) < 0) {

+    if ((len = virFileReadHeaderFD(fd, len, (char **)&head)) < 0) {

         virReportSystemError(errno, _("cannot read header '%s'"), path);

         goto cleanup;

     }

-- 

1.9.0