From 5bb45373d483fc70fca59ff4244c48722ce75141 Mon Sep 17 00:00:00 2001
From: Dan Winship <danw@gnome.org>
Date: Mon, 17 Mar 2014 13:32:18 -0400
Subject: [PATCH] ifcfg-rh: fix up lowlevel ifcfg file reading code

shvar.c was assuming it could do a single read() to read in the ifcfg
file, without taking partial reads or EINTR into account. Fix that.

Also, it was keeping the raw contents of the ifcfg file in the
shvarFile even though it never looked at it after svOpenFile().
(Presumably lineList originally consisted of pointers into arena, but
that had to be changed to support readwrite.) Fix that.

It would simplify things further to use g_file_get_contents() and
g_file_set_contents(), but the current code is perhaps more resilient
to symlink attacks because it keeps the fd open?
---
 src/settings/plugins/ifcfg-rh/shvar.c | 21 +++++++++++++++------
 src/settings/plugins/ifcfg-rh/shvar.h |  1 -
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c
index c6c6808e45..db1ed4da37 100644
--- a/src/settings/plugins/ifcfg-rh/shvar.c
+++ b/src/settings/plugins/ifcfg-rh/shvar.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -56,17 +57,27 @@ svOpenFile(const char *name, gboolean create)
 
     if (s->fd != -1) {
 	struct stat buf;
-	char *p, *q;
+	char *arena, *p, *q;
+	ssize_t nread, total = 0;
 
 	if (fstat(s->fd, &buf) < 0) goto bail;
-	s->arena = g_malloc0(buf.st_size + 1);
+	arena = g_malloc(buf.st_size + 1);
+	arena[buf.st_size] = '\0';
 
-	if (read(s->fd, s->arena, buf.st_size) < 0) goto bail;
+	while (total < buf.st_size) {
+		nread = read(s->fd, arena + total, buf.st_size - total);
+		if (nread == -1 && errno == EINTR)
+			continue;
+		if (nread <= 0)
+			goto bail;
+		total += nread;
+	}
 
 	/* we'd use g_strsplit() here, but we want a list, not an array */
-	for(p = s->arena; (q = strchr(p, '\n')) != NULL; p = q + 1) {
+	for(p = arena; (q = strchr(p, '\n')) != NULL; p = q + 1) {
 		s->lineList = g_list_append(s->lineList, g_strndup(p, q - p));
 	}
+	g_free (arena);
 
 	/* closefd is set if we opened the file read-only, so go ahead and
 	   close it, because we can't write to it anyway */
@@ -84,7 +95,6 @@ svOpenFile(const char *name, gboolean create)
 
 bail:
     if (s->fd != -1) close(s->fd);
-    g_free (s->arena);
     g_free (s->fileName);
     g_free (s);
     return NULL;
@@ -374,7 +384,6 @@ svCloseFile(shvarFile *s)
 
     if (s->fd != -1) close(s->fd);
 
-    g_free(s->arena);
     g_free(s->fileName);
     g_list_free_full (s->lineList, g_free); /* implicitly frees s->current */
     g_free(s);
diff --git a/src/settings/plugins/ifcfg-rh/shvar.h b/src/settings/plugins/ifcfg-rh/shvar.h
index 4ccf83bb81..b55cf06b39 100644
--- a/src/settings/plugins/ifcfg-rh/shvar.h
+++ b/src/settings/plugins/ifcfg-rh/shvar.h
@@ -40,7 +40,6 @@ typedef struct _shvarFile shvarFile;
 struct _shvarFile {
 	char		*fileName;	/* read-only */
 	int		fd;		/* read-only */
-	char		*arena;		/* ignore */
 	GList		*lineList;	/* read-only */
 	GList		*current;	/* set implicitly or explicitly,
 					   points to element of lineList */