Re: [PATCH] scripts/common: fix write_file when opened with mmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sascha,

On 31. 01. 22 10:23, Sascha Hauer wrote:
Hi Andrej,

On Fri, Jan 28, 2022 at 09:00:33AM +0100, Andrej Picej wrote:
Usage of bareboximd with -c option (Create checksum for FILE and write
it to the crc32 tag) was broken. Possibly by applying 2154de1cf36c
(bareboximd: Use mmap when possibly). The script fails with:

     $ ./scripts/bareboximd -c images/<barebox-image.img>
     Cannot write to images/<barebox-mage.img>: Bad address
     CRC: write crc token to images/<barebox-image.img> failed: -14
     Bad address

This has to do with the usage of "mmap" and "open" with O_TRUNC flag
which truncates the file length to 0. Writing to files fails with:
EFAULT (14) buf is outside your accessible address space.
Remove the truncate flag and truncate manually after writing the data.

This fixes the bareboximd script, which is now again usable with -c
option.

That's a very unfortunate bug :(


Signed-off-by: Anze Lesnik <anze.lesnik@xxxxxxxxx>
Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx>
---
  scripts/common.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/common.c b/scripts/common.c
index 3f8dcd2d38..331988d593 100644
--- a/scripts/common.c
+++ b/scripts/common.c
@@ -107,7 +107,7 @@ int write_file(const char *filename, const void *buf, size_t size)
  	int fd, ret = 0;
  	int now;
- fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT,

I added a comment above this while applying to prevent people from
"optimizing" this patch away.

+	fd = open(filename, O_WRONLY | O_CREAT,
  		  S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
  	if (fd < 0) {
  		fprintf(stderr, "Cannot open %s: %s\n", filename, strerror(errno));
@@ -126,6 +126,11 @@ int write_file(const char *filename, const void *buf, size_t size)
  		buf += now;
  	}
+ if (ftruncate(fd, now) < 0) {
+		fprintf(stderr, "Cannot truncate file: %s", strerror(errno));
+		ret = -errno;
+	}

truncating to 'now' is only correct when we were able to write all bytes
at once. Otherwise we must truncate to the input size. I fixed this up
to the patch below.

That makes sense. Good thing you caught that.

Thanks.


Sascha

-----------------------------8<----------------------------

 From 738601e1258c55953284ee10801b26b9977918c2 Mon Sep 17 00:00:00 2001
From: Andrej Picej <andrej.picej@xxxxxxxxx>
Date: Fri, 28 Jan 2022 09:00:33 +0100
Subject: [PATCH] scripts/common: fix write_file when opened with mmap

Usage of bareboximd with -c option (Create checksum for FILE and write
it to the crc32 tag) was broken. Possibly by applying 2154de1cf36c
(bareboximd: Use mmap when possibly). The script fails with:

     $ ./scripts/bareboximd -c images/<barebox-image.img>
     Cannot write to images/<barebox-mage.img>: Bad address
     CRC: write crc token to images/<barebox-image.img> failed: -14
     Bad address

This has to do with the usage of "mmap" and "open" with O_TRUNC flag
which truncates the file length to 0. Writing to files fails with:
EFAULT (14) buf is outside your accessible address space.
Remove the truncate flag and truncate manually after writing the data.

This fixes the bareboximd script, which is now again usable with -c
option.

Signed-off-by: Anze Lesnik <anze.lesnik@xxxxxxxxx>
Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx>
Link: https://lore.barebox.org/20220128080033.167251-1-andrej.picej@xxxxxxxxx
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
  scripts/common.c | 15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/scripts/common.c b/scripts/common.c
index b780b09941..154d6dffcb 100644
--- a/scripts/common.c
+++ b/scripts/common.c
@@ -105,26 +105,33 @@ int write_file(const char *filename, const void *buf, size_t size)
  {
  	int fd, ret = 0;
  	int now;
+	size_t left = size;
- fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT,
+	/* The same file may be mmapped currently, so can't use O_TRUNC here */
+	fd = open(filename, O_WRONLY | O_CREAT,
  		  S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
  	if (fd < 0) {
  		fprintf(stderr, "Cannot open %s: %s\n", filename, strerror(errno));
  		return -errno;
  	}
- while (size) {
-		now = write(fd, buf, size);
+	while (left) {
+		now = write(fd, buf, left);
  		if (now < 0) {
  			fprintf(stderr, "Cannot write to %s: %s\n", filename,
  				strerror(errno));
  			ret = -errno;
  			goto out;
  		}
-		size -= now;
+		left -= now;
  		buf += now;
  	}
+ if (ftruncate(fd, size) < 0) {
+		fprintf(stderr, "Cannot truncate file: %s", strerror(errno));
+		ret = -errno;
+	}
+
  out:
  	close(fd);

_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox



[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux