fileio: Unify whole-file reads

We do whole-file reads in a few places, so unify to a fileio_read_file()
function.

To do this, we change the type of struct image->buf to a uint8_t *.
Where we do pointer manipulation on the image buffer, we need a
temporary void * variable.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

fileio.c

@@ -31,11 +31,18 @@
  */
 
 #include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/stat.h>
 
 #include <openssl/bio.h>
 #include <openssl/pem.h>
 #include <openssl/err.h>
 
+#include <ccan/talloc/talloc.h>
+#include <ccan/read_write_all/read_write_all.h>
+
 #include "fileio.h"
 
 EVP_PKEY *fileio_read_pkey(const char *filename)
@@ -78,3 +85,53 @@ out:
 	}
 	return cert;
 }
+
+int fileio_read_file(void *ctx, const char *filename,
+		 uint8_t **out_buf, size_t *out_len)
+{
+	struct stat statbuf;
+	uint8_t *buf;
+	size_t len;
+	int fd, rc;
+
+	rc = -1;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		perror("open");
+		goto out;
+	}
+
+	rc = fstat(fd, &statbuf);
+	if (rc) {
+		perror("fstat");
+		goto out;
+	}
+
+	len = statbuf.st_size;
+
+	buf = talloc_array(ctx, uint8_t, len);
+	if (!buf) {
+		perror("talloc");
+		goto out;
+	}
+
+	if (!read_all(fd, buf, len)) {
+		perror("read_all");
+		goto out;
+	}
+
+	rc = 0;
+
+out:
+	if (fd >= 0)
+		close(fd);
+	if (rc) {
+		fprintf(stderr, "Error reading file %s\n", filename);
+	} else {
+		*out_buf = buf;
+		*out_len = len;
+	}
+	return rc;
+
+}

fileio.h

@@ -32,11 +32,16 @@
 #ifndef FILEIO_H
 #define FILEIO_H
 
+#include <stdint.h>
+
 #include <openssl/evp.h>
 #include <openssl/x509.h>
 
 EVP_PKEY *fileio_read_pkey(const char *filename);
 X509 *fileio_read_cert(const char *filename);
 
+int fileio_read_file(void *ctx, const char *filename,
+		uint8_t **out_buf, size_t *out_len);
+
 #endif /* FILEIO_H */
 

image.c

@@ -43,6 +43,7 @@
 #include <ccan/build_assert/build_assert.h>
 #include <openssl/sha.h>
 
+#include "fileio.h"
 #include "image.h"
 
 #define DATA_DIR_CERT_TABLE	4
@@ -81,6 +82,7 @@ static int image_pecoff_parse(struct image *image)
 	struct cert_table_header *cert_table;
 	char nt_sig[] = {'P', 'E', 0, 0};
 	size_t size = image->size;
+	void *buf = image->buf;
 	uint32_t addr;
 
 	/* sanity checks */
@@ -89,7 +91,7 @@ static int image_pecoff_parse(struct image *image)
 		return -1;
 	}
 
-	image->doshdr = image->buf;
+	image->doshdr = buf;
 
 	if (image->doshdr->e_magic[0] != 0x4d
 			|| image->doshdr->e_magic[1] != 0x5a) {
@@ -109,7 +111,7 @@ static int image_pecoff_parse(struct image *image)
 		return -1;
 	}
 
-	image->pehdr = image->buf + addr;
+	image->pehdr = buf + addr;
 	if (memcmp(image->pehdr->nt_signature, nt_sig, sizeof(nt_sig))) {
 		fprintf(stderr, "Invalid PE header signature\n");
 		return -1;
@@ -146,7 +148,7 @@ static int image_pecoff_parse(struct image *image)
 
 	image->cert_table_size = image->data_dir_sigtable->size;
 	if (image->cert_table_size)
-		cert_table = image->buf + image->data_dir_sigtable->addr;
+		cert_table = buf + image->data_dir_sigtable->addr;
 	else
 		cert_table = NULL;
 
@@ -170,7 +172,6 @@ static int image_pecoff_parse(struct image *image)
 
 struct image *image_load(const char *filename)
 {
-	struct stat statbuf;
 	struct image *image;
 	int rc;
 
@@ -180,33 +181,9 @@ struct image *image_load(const char *filename)
 		return NULL;
 	}
 
-	image->fd = open(filename, O_RDONLY);
+	rc = fileio_read_file(image, filename, &image->buf, &image->size);
-	if (image->fd < 0) {
+	if (rc)
-		perror("open");
 		goto err;
-	}
-
-	rc = fstat(image->fd, &statbuf);
-	if (rc) {
-		perror("fstat");
-		goto err;
-	}
-
-	image->size = statbuf.st_size;
-
-	image->buf = talloc_size(image, image->size);
-	if (!image->buf) {
-		perror("talloc(buf)");
-		goto err;
-	}
-
-	if (!read_all(image->fd, image->buf, image->size)) {
-		perror("read_all");
-		fprintf(stderr, "error reading input file\n");
-		goto err;
-	}
-
-	lseek(image->fd, 0, SEEK_SET);
 
 	rc = image_pecoff_parse(image);
 	if (rc)
@@ -243,6 +220,7 @@ static void set_region_from_range(struct region *region, void *start, void *end)
 int image_find_regions(struct image *image)
 {
 	struct region *regions;
+	void *buf = image->buf;
 	int i, gap_warn;
 	uint32_t align;
 	size_t bytes;
@@ -263,7 +241,7 @@ int image_find_regions(struct image *image)
 
 	/* first region: beginning to checksum field */
 	regions = image->checksum_regions;
-	set_region_from_range(&regions[0], image->buf, image->checksum);
+	set_region_from_range(&regions[0], buf, image->checksum);
 	regions[0].name = "begin->cksum";
 	bytes += regions[0].size;
 
@@ -282,8 +260,7 @@ int image_find_regions(struct image *image)
 	set_region_from_range(&regions[2],
 				(void *)image->data_dir_sigtable
 					+ sizeof(struct data_dir_entry),
-				image->buf +
+				buf + pehdr_u32(image->aouthdr->SizeOfHeaders));
-				pehdr_u32(image->aouthdr->SizeOfHeaders));
 	regions[2].name = "datadir[CERT]->headers";
 	bytes += regions[2].size;
 
@@ -304,7 +281,7 @@ int image_find_regions(struct image *image)
 				image->n_checksum_regions);
 		regions = image->checksum_regions;
 
-		regions[i + 3].data = image->buf + file_offset;
+		regions[i + 3].data = buf + file_offset;
 		regions[i + 3].size = align_up(file_size, align);
 		regions[i + 3].name = talloc_strndup(image->checksum_regions,
 					image->scnhdr[i].s_name, 8);
@@ -315,14 +292,14 @@ int image_find_regions(struct image *image)
 			fprintf(stderr, "warning: gap in section table:\n");
 			fprintf(stderr, "    %-8s: 0x%08tx - 0x%08tx,\n",
 					regions[i+2].name,
-					regions[i+2].data - image->buf,
+					regions[i+2].data - buf,
 					regions[i+2].data +
-						regions[i+2].size - image->buf);
+						regions[i+2].size - buf);
 			fprintf(stderr, "    %-8s: 0x%08tx - 0x%08tx,\n",
 					regions[i+3].name,
-					regions[i+3].data - image->buf,
+					regions[i+3].data - buf,
 					regions[i+3].data +
-						regions[i+3].size - image->buf);
+						regions[i+3].size - buf);
 
 
 			gap_warn = 1;

image.h

@@ -48,8 +48,7 @@ struct region {
 };
 
 struct image {
-	int		fd;
+	uint8_t		*buf;
-	void		*buf;
 	size_t		size;
 
 	/* Pointers to interesting parts of the image */

sbattach.c

@@ -54,6 +54,7 @@
 #include "config.h"
 
 #include "image.h"
+#include "fileio.h"
 
 static const char *toolname = "sbattach";
 
@@ -96,45 +97,19 @@ static int detach_sig(struct image *image, const char *sig_filename)
 static int attach_sig(struct image *image, const char *image_filename,
 		const char *sig_filename)
 {
-	struct stat statbuf;
+	const uint8_t *tmp_buf;
 	uint8_t *sigbuf;
 	size_t size;
-	int fd, rc;
 	PKCS7 *p7;
-	const uint8_t *tmp_buf;
+	int rc;
 
-	sigbuf = NULL;
+	rc = fileio_read_file(image, sig_filename, &sigbuf, &size);
-
+	if (rc)
-	fd = open(sig_filename, O_RDONLY);
-	if (fd < 0) {
-		fprintf(stderr, "Can't open file %s: %s\n", sig_filename,
-				strerror(errno));
-		return -1;
-	}
-
-	rc = fstat(fd, &statbuf);
-	if (rc) {
-		perror("fstat");
 		goto out;
-	}
-
-	size = statbuf.st_size;
-
-	sigbuf = talloc_array(image, uint8_t, size);
-	if (!sigbuf) {
-		perror("talloc");
-		goto out;
-	}
-
-	rc = read_all(fd, sigbuf, size);
-	if (!rc) {
-		fprintf(stderr, "Error reading %s: %s\n", sig_filename,
-				strerror(errno));
-		goto out;
-	}
 
 	image_add_signature(image, sigbuf, size);
 
+	rc = -1;
 	tmp_buf = sigbuf;
 	p7 = d2i_PKCS7(NULL, &tmp_buf, size);
 	if (!p7) {
@@ -158,7 +133,6 @@ static int attach_sig(struct image *image, const char *image_filename,
 				strerror(errno));
 
 out:
-	close(fd);
 	talloc_free(sigbuf);
 	return rc;
 }

sbvarsign.c

@@ -65,7 +65,7 @@ struct varsign_context {
 	const char			*outfilename;
 
 	uint8_t				*data;
-	unsigned int			data_len;
+	size_t				data_len;
 
 	CHAR16				*var_name;
 	int				var_name_bytes;
@@ -193,50 +193,6 @@ static int parse_guid(const char *str, EFI_GUID *guid)
 	return 0;
 }
 
-static int read_var_data(struct varsign_context *ctx)
-{
-	struct stat statbuf;
-	int rc, fd = -1;
-
-	fd = open(ctx->infilename, O_RDONLY);
-	if (fd < 0) {
-		perror("open");
-		goto err;
-	}
-
-	rc = fstat(fd, &statbuf);
-	if (rc) {
-		perror("fstat");
-		goto err;
-	}
-
-	ctx->data_len = statbuf.st_size;
-	ctx->data = talloc_size(ctx, ctx->data_len);
-
-	if (!ctx->data) {
-		perror("talloc");
-		goto err;
-	}
-
-	if (!read_all(fd, ctx->data, ctx->data_len)) {
-		perror("read_all");
-		goto err;
-	}
-
-	close(fd);
-	return 0;
-
-err:
-	if (fd > 0)
-		close(fd);
-	ctx->data_len = 0;
-	talloc_free(ctx->data);
-	ctx->data = NULL;
-	fprintf(stderr, "Can't read variable data from file %s\n",
-			ctx->infilename);
-	return -1;
-}
-
 static int set_timestamp(EFI_TIME *timestamp)
 {
 	struct tm *tm;
@@ -554,7 +510,7 @@ int main(int argc, char **argv)
 		ctx->var_guid = default_guid;
 	}
 
-	if (read_var_data(ctx))
+	if (fileio_read_file(ctx, ctx->infilename, &ctx->data, &ctx->data_len))
 		return EXIT_FAILURE;
 
 	ctx->key = fileio_read_pkey(keyfilename);

sbverify.c

@@ -111,7 +111,7 @@ static int load_image_signature_data(struct image *image,
 		return -1;
 	}
 
-	header = image->buf + image->data_dir_sigtable->addr;
+	header = (void *)image->buf + image->data_dir_sigtable->addr;
 	*buf = (void *)(header + 1);
 	*len = header->size - sizeof(*header);
 	return 0;
@@ -120,43 +120,7 @@ static int load_image_signature_data(struct image *image,
 static int load_detached_signature_data(struct image *image,
 		const char *filename, uint8_t **buf, size_t *len)
 {
-	struct stat statbuf;
+	return fileio_read_file(image, filename, buf, len);
-	uint8_t *tmpbuf = NULL;
-	int fd, rc;
-
-	fd = open(filename, O_RDONLY);
-	if (fd < 0) {
-		fprintf(stderr, "Couldn't open %s: %s\n", filename,
-				strerror(errno));
-		return -1;
-	}
-
-	rc = fstat(fd, &statbuf);
-	if (rc) {
-		perror("stat");
-		goto err;
-	}
-
-	tmpbuf = talloc_array(image, uint8_t, statbuf.st_size);
-	if (!tmpbuf) {
-		perror("talloc_array");
-		goto err;
-	}
-
-	rc = read_all(fd, tmpbuf, statbuf.st_size);
-	if (!rc) {
-		perror("read_all");
-		goto err;
-	}
-
-	*buf = tmpbuf;
-	*len = statbuf.st_size;
-	return 0;
-
-err:
-	close(fd);
-	talloc_free(tmpbuf);
-	return -1;
 }
 
 static int x509_verify_cb(int status, X509_STORE_CTX *ctx)