Author: stefanct Date: Fri Dec 25 22:59:45 2015 New Revision: 1902 URL: http://flashrom.org/trac/flashrom/changeset/1902
Log: Rigorously check integrity of I/O stream data.
Even if fwrite() succeeds the data is not necessarily out of the clib's buffers and writing it eventually could fail. Even if the data is flushed out (explicitly by fflush() or implicitly by fclose()) the kernel might still hold a buffer.
Previously we have ignored this to a large extent - even in important cases like writing the flash contents to a file. The results can be truncated images that would brick the respective machine if written back as is (though flashrom would not allow that due to a size mismatch). flashrom would not indicate the problem in any output - so far we only check the return value of fwrite() that is not conclusive.
This patch checks the return values of all related system calls like fclose() unless we only read the file and are not really interested in output errors. In the latter case the return value is casted to void to document this fact. Additionally, this patch explicitly calls fflush() and fsync() (on regular files only) to do the best we can to guarantee the read image reaches the disk safely and at least inform the user if it did not work.
Signed-off-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at Acked-by: Urja Rannikko urjaman@gmail.com
Modified: trunk/flashrom.c trunk/layout.c trunk/processor_enable.c
Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Sun Nov 22 03:23:31 2015 (r1901) +++ trunk/flashrom.c Fri Dec 25 22:59:45 2015 (r1902) @@ -28,6 +28,7 @@ #include <sys/stat.h> #endif #include <string.h> +#include <unistd.h> #include <stdlib.h> #include <errno.h> #include <ctype.h> @@ -1255,36 +1256,36 @@ msg_gerr("Error: No file I/O support in libpayload\n"); return 1; #else - unsigned long numbytes; - FILE *image; - struct stat image_stat; + int ret = 0;
+ FILE *image; if ((image = fopen(filename, "rb")) == NULL) { msg_gerr("Error: opening file "%s" failed: %s\n", filename, strerror(errno)); return 1; } + + struct stat image_stat; if (fstat(fileno(image), &image_stat) != 0) { msg_gerr("Error: getting metadata of file "%s" failed: %s\n", filename, strerror(errno)); - fclose(image); - return 1; + ret = 1; + goto out; } if (image_stat.st_size != size) { msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n", (intmax_t)image_stat.st_size, size); - fclose(image); - return 1; - } - numbytes = fread(buf, 1, size, image); - if (fclose(image)) { - msg_gerr("Error: closing file "%s" failed: %s\n", filename, strerror(errno)); - return 1; + ret = 1; + goto out; } + + unsigned long numbytes = fread(buf, 1, size, image); if (numbytes != size) { msg_gerr("Error: Failed to read complete file. Got %ld bytes, " "wanted %ld!\n", numbytes, size); - return 1; + ret = 1; } - return 0; +out: + (void)fclose(image); + return ret; #endif }
@@ -1294,8 +1295,8 @@ msg_gerr("Error: No file I/O support in libpayload\n"); return 1; #else - unsigned long numbytes; FILE *image; + int ret = 0;
if (!filename) { msg_gerr("No filename specified.\n"); @@ -1306,14 +1307,37 @@ return 1; }
- numbytes = fwrite(buf, 1, size, image); - fclose(image); + unsigned long numbytes = fwrite(buf, 1, size, image); if (numbytes != size) { - msg_gerr("File %s could not be written completely.\n", - filename); - return 1; + msg_gerr("Error: file %s could not be written completely.\n", filename); + ret = 1; + goto out; + } + if (fflush(image)) { + msg_gerr("Error: flushing file "%s" failed: %s\n", filename, strerror(errno)); + ret = 1; } - return 0; + // Try to fsync() only regular files and if that function is available at all (e.g. not on MinGW). +#if defined(_POSIX_FSYNC) && (_POSIX_FSYNC != -1) + struct stat image_stat; + if (fstat(fileno(image), &image_stat) != 0) { + msg_gerr("Error: getting metadata of file "%s" failed: %s\n", filename, strerror(errno)); + ret = 1; + goto out; + } + if (S_ISREG(image_stat.st_mode)) { + if (fsync(fileno(image))) { + msg_gerr("Error: fsyncing file "%s" failed: %s\n", filename, strerror(errno)); + ret = 1; + } + } +#endif +out: + if (fclose(image)) { + msg_gerr("Error: closing file "%s" failed: %s\n", filename, strerror(errno)); + ret = 1; + } + return ret; #endif }
Modified: trunk/layout.c ============================================================================== --- trunk/layout.c Sun Nov 22 03:23:31 2015 (r1901) +++ trunk/layout.c Fri Dec 25 22:59:45 2015 (r1902) @@ -65,7 +65,7 @@ if (num_rom_entries >= MAX_ROMLAYOUT) { msg_gerr("Maximum number of ROM images (%i) in layout " "file reached.\n", MAX_ROMLAYOUT); - fclose(romlayout); + (void)fclose(romlayout); return 1; } if (2 != fscanf(romlayout, "%s %s\n", tempstr, rom_entries[num_rom_entries].name)) @@ -80,7 +80,7 @@ tstr2 = strtok(NULL, ":"); if (!tstr1 || !tstr2) { msg_gerr("Error parsing layout file. Offending string: "%s"\n", tempstr); - fclose(romlayout); + (void)fclose(romlayout); return 1; } rom_entries[num_rom_entries].start = strtol(tstr1, (char **)NULL, 16); @@ -95,7 +95,7 @@ rom_entries[i].end, rom_entries[i].name); }
- fclose(romlayout); + (void)fclose(romlayout);
return 0; }
Modified: trunk/processor_enable.c ============================================================================== --- trunk/processor_enable.c Sun Nov 22 03:23:31 2015 (r1901) +++ trunk/processor_enable.c Fri Dec 25 22:59:45 2015 (r1902) @@ -57,11 +57,11 @@ ptr++; while (*ptr && isspace((unsigned char)*ptr)) ptr++; - fclose(cpuinfo); + (void)fclose(cpuinfo); return (strncmp(ptr, "ICT Loongson-2 V0.3", strlen("ICT Loongson-2 V0.3")) == 0) || (strncmp(ptr, "Godson2 V0.3 FPU V0.1", strlen("Godson2 V0.3 FPU V0.1")) == 0); } - fclose(cpuinfo); + (void)fclose(cpuinfo); return 0; } #endif