[Xcb-commit] xcb/util-cursor: cursor

[Michael Stapelberg]

 cursor/parse_cursor_file.c |   70 ++++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 26 deletions(-)

New commits:
commit c453d4baceaead514c814b825cdd9b517c6bcd8c
Author: Michael Stapelberg <michael at stapelberg.de>
Date:   Thu Nov 7 22:00:21 2013 +0100

    handle read() errors (Thanks psychon)

diff --git a/cursor/parse_cursor_file.c b/cursor/parse_cursor_file.c
index 30f7df5..b944c4a 100644
--- a/cursor/parse_cursor_file.c
+++ b/cursor/parse_cursor_file.c
@@ -35,6 +35,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <stdbool.h>
 
 #ifdef HAVE_ENDIAN_H
 #include <endian.h>
@@ -82,14 +83,24 @@ static uint32_t find_best_size(xcint_cursor_file_t *cf, const uint32_t target, u
     return best;
 }
 
+/* Returns true if and only if the read() call read the entirety of the data it
+ * was supposed to read. */
+static bool read_entirely(int fd, void *buf, size_t count) {
+    return read(fd, buf, count) == count;
+}
+
 int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **images, int *nimg) {
     /* Read the header, verify the magic value. */
     xcint_cursor_file_t cf;
     uint32_t nsizes = 0;
     uint32_t best = 0;
     uint32_t skip = 0;
+    /* The amount of images stored in 'images', used when cleaning up. */
+    int cnt = 0;
+
+    if (!read_entirely(fd, &(cf.header), sizeof(xcint_file_header_t)))
+        return -EINVAL;
 
-    read(fd, &(cf.header), sizeof(xcint_file_header_t));
     cf.header.magic = le32toh(cf.header.magic);
     cf.header.header = le32toh(cf.header.header);
     cf.header.version = le32toh(cf.header.version);
@@ -107,7 +118,9 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
 
     /* Read the table of contents */
     cf.tocs = malloc(cf.header.ntoc * sizeof(xcint_file_toc_t));
-    read(fd, cf.tocs, cf.header.ntoc * sizeof(xcint_file_toc_t));
+    if (!read_entirely(fd, cf.tocs, cf.header.ntoc * sizeof(xcint_file_toc_t)))
+        goto error;
+
     for (int n = 0; n < cf.header.ntoc; n++) {
         cf.tocs[n].type = le32toh(cf.tocs[n].type);
         cf.tocs[n].subtype = le32toh(cf.tocs[n].subtype);
@@ -115,23 +128,17 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
     }
 
     /* No images? Invalid file. */
-    if ((best = find_best_size(&cf, c->size, &nsizes)) == 0 || nsizes == 0) {
-        free(cf.tocs);
-        return -EINVAL;
-    }
+    if ((best = find_best_size(&cf, c->size, &nsizes)) == 0 || nsizes == 0)
+        goto error;
 
     *nimg = nsizes;
-    if ((*images = calloc(nsizes, sizeof(xcint_image_t))) == NULL) {
-        free(cf.tocs);
-        return -errno;
-    }
+    if ((*images = calloc(nsizes, sizeof(xcint_image_t))) == NULL)
+        goto error;
 
-    for (int n = 0, cnt = 0;
-         n < cf.header.ntoc;
-         n++) {
+    for (int n = 0; n < cf.header.ntoc; n++) {
         xcint_chunk_header_t chunk;
         /* for convenience */
-        xcint_image_t *i = &((*images)[cnt++]);
+        xcint_image_t *i = &((*images)[n]);
         uint32_t numpixels = 0;
         uint32_t *p = NULL;
 
@@ -140,18 +147,18 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
             continue;
 
         lseek(fd, cf.tocs[n].position, SEEK_SET);
-        read(fd, &chunk, sizeof(xcint_chunk_header_t));
+        if (!read_entirely(fd, &chunk, sizeof(xcint_chunk_header_t)))
+            goto error2;
         chunk.header = le32toh(chunk.header);
         chunk.type = le32toh(chunk.type);
         chunk.subtype = le32toh(chunk.subtype);
         chunk.version = le32toh(chunk.version);
         /* Sanity check, as libxcursor does it. */
         if (chunk.type != cf.tocs[n].type ||
-            chunk.subtype != cf.tocs[n].subtype) {
-            free(cf.tocs);
-            return -EINVAL;
-        }
-        read(fd, i, sizeof(xcint_image_t) - sizeof(uint32_t*)); // TODO: better type
+            chunk.subtype != cf.tocs[n].subtype)
+            goto error2;
+        if (!read_entirely(fd, i, sizeof(xcint_image_t) - sizeof(uint32_t*))) // TODO: better type
+            goto error2;
         i->width = le32toh(i->width);
         i->height = le32toh(i->height);
         i->xhot = le32toh(i->xhot);
@@ -159,14 +166,15 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
         i->delay = le32toh(i->delay);
 
         /* Read the actual image data and convert it to host byte order */
-        if (((uint64_t)i->width) * i->height > UINT32_MAX) {
-            /* Catch integer overflows */
-            free(cf.tocs);
-            return -EINVAL;
-        }
+        /* Catch integer overflows */
+        if (((uint64_t)i->width) * i->height > UINT32_MAX)
+            goto error2;
         numpixels = i->width * i->height;
         i->pixels = malloc(numpixels * sizeof(uint32_t));
-        read(fd, i->pixels, numpixels * sizeof(uint32_t));
+        /* With the malloc, one more image is eligible for cleanup later. */
+        cnt++;
+        if (!read_entirely(fd, i->pixels, numpixels * sizeof(uint32_t)))
+            goto error2;
         p = i->pixels;
         for (int j = 0; j < numpixels; j++, p++)
             *p = le32toh(*p);
@@ -174,4 +182,14 @@ int parse_cursor_file(xcb_cursor_context_t *c, const int fd, xcint_image_t **ima
 
     free(cf.tocs);
     return 0;
+
+error2:
+    /* Free the memory for all images that were read so far. */
+    for (int n = 0; n < cnt; n++)
+        free((*images)[n].pixels);
+    free(*images);
+error:
+    *images = NULL;
+    free(cf.tocs);
+    return -EINVAL;
 }