[coreboot-gerrit] Patch set updated for coreboot: cbfstool: accept read-only files when possible

[Patrick Georgi (pgeorgi@google.com)]

Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12931

-gerrit

commit 2f43b0e3d0c5b9364e5b54dfdafc80f53a488dd0
Author: Vadim Bendebury <vbendeb at chromium.org>
Date:   Wed Dec 9 09:39:31 2015 -0800

    cbfstool: accept read-only files when possible
    
    cbfstool tries opening the input file for write access even if the
    command does not require modifying the file.
    
    Let's not request write access unless it is necessary, this way one
    can examine write protected files without sudo.
    
    BRANCH=none
    BUG=none
    TEST=running
    
       cbfstool /build/<board>/firmware/image.bin print
    
       in chroot does not require root access any more.
    
    Change-Id: Ic4e4cc389b160da190e44a676808f5c4e6625567
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: ef6a8e25d9e257d7de4cc6b94e510234fe20a56d
    Original-Change-Id: I871f32f0662221ffbdb13bf0482cb285ec184d07
    Original-Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/317300
    Original-Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
 util/cbfstool/cbfstool.c         | 13 +++++++++----
 util/cbfstool/partitioned_file.c | 15 ++++++++++-----
 util/cbfstool/partitioned_file.h |  4 +++-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 62edd4b..8ad66c4 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -38,7 +38,10 @@ struct command {
 	int (*function) (void);
 	// Whether to populate param.image_region before invoking function
 	bool accesses_region;
-	// Whether to write that region's contents back to image_file at the end
+	// This set to true means two things:
+	// - in case of a command operating on a region, the region's contents
+	//   will be written back to image_file at the end
+	// - write access to the file is required
 	bool modifies_region;
 };
 
@@ -1014,7 +1017,7 @@ static const struct command commands[] = {
 	{"print", "H:r:vh?", cbfs_print, true, false},
 	{"read", "r:f:vh?", cbfs_read, true, false},
 	{"remove", "H:r:n:vh?", cbfs_remove, true, true},
-	{"update-fit", "H:r:n:x:vh?", cbfs_update_fit, true, false},
+	{"update-fit", "H:r:n:x:vh?", cbfs_update_fit, true, true},
 	{"write", "r:f:udvh?", cbfs_write, true, true},
 };
 
@@ -1372,8 +1375,11 @@ int main(int argc, char **argv)
 				return 1;
 			}
 		} else {
+			bool write_access = commands[i].modifies_region;
+
 			param.image_file =
-				partitioned_file_reopen(image_name);
+				partitioned_file_reopen(image_name,
+							write_access);
 		}
 		if (!param.image_file)
 			return 1;
@@ -1429,7 +1435,6 @@ int main(int argc, char **argv)
 
 		if (commands[i].modifies_region) {
 			assert(param.image_file);
-			assert(commands[i].accesses_region);
 			for (unsigned region = 0; region < num_regions;
 								++region) {
 
diff --git a/util/cbfstool/partitioned_file.c b/util/cbfstool/partitioned_file.c
index f019d71..7b4b003 100644
--- a/util/cbfstool/partitioned_file.c
+++ b/util/cbfstool/partitioned_file.c
@@ -49,11 +49,13 @@ static unsigned count_selected_fmap_entries(const struct fmap *fmap,
 	return count;
 }
 
-static partitioned_file_t *reopen_flat_file(const char *filename)
+static partitioned_file_t *reopen_flat_file(const char *filename,
+					    bool write_access)
 {
 	assert(filename);
-
 	struct partitioned_file *file = calloc(1, sizeof(*file));
+	const char *access_mode;
+
 	if (!file) {
 		ERROR("Failed to allocate partitioned file structure\n");
 		return NULL;
@@ -64,7 +66,9 @@ static partitioned_file_t *reopen_flat_file(const char *filename)
 		return NULL;
 	}
 
-	file->stream = fopen(filename, "rb+");
+	access_mode = write_access ?  "rb+" : "rb";
+	file->stream = fopen(filename, access_mode);
+
 	if (!file->stream) {
 		perror(filename);
 		partitioned_file_close(file);
@@ -161,11 +165,12 @@ partitioned_file_t *partitioned_file_create(const char *filename,
 	return file;
 }
 
-partitioned_file_t *partitioned_file_reopen(const char *filename)
+partitioned_file_t *partitioned_file_reopen(const char *filename,
+					    bool write_access)
 {
 	assert(filename);
 
-	partitioned_file_t *file = reopen_flat_file(filename);
+	partitioned_file_t *file = reopen_flat_file(filename, write_access);
 	if (!file)
 		return NULL;
 
diff --git a/util/cbfstool/partitioned_file.h b/util/cbfstool/partitioned_file.h
index 2fc2bfe..5a8f4dd 100644
--- a/util/cbfstool/partitioned_file.h
+++ b/util/cbfstool/partitioned_file.h
@@ -71,9 +71,11 @@ partitioned_file_t *partitioned_file_create(const char *filename,
  * caller, and must later be passed to partitioned_file_close();
  *
  * @param filename      Name of the file to read in
+ * @param write_access  True if the file needs to be modified
  * @return              Caller-owned partitioned file, or NULL on error
  */
-partitioned_file_t *partitioned_file_reopen(const char *filename);
+partitioned_file_t *partitioned_file_reopen(const char *filename,
+					    bool write_access);
 
 /**
  * Write a buffer's contents to its original region within a segmented file.