diff mbox

[U-Boot,v2,4/7] x86: ifdtool: Check that U-Boot does not overlap other regions

Message ID 1439671074-6683-5-git-send-email-sjg@chromium.org
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Aug. 15, 2015, 8:37 p.m. UTC 
Since U-Boot and its device tree can grow we should check that it does not
overlap the regions above it. Track the ROM offset that U-Boot reaches and
check that other regions (written after U-Boot) do not interfere.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 tools/ifdtool.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Bin Meng Aug. 19, 2015, 2:33 p.m. UTC | #1 
On Sun, Aug 16, 2015 at 4:37 AM, Simon Glass <sjg@chromium.org> wrote:
> Since U-Boot and its device tree can grow we should check that it does not
> overlap the regions above it. Track the ROM offset that U-Boot reaches and
> check that other regions (written after U-Boot) do not interfere.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Nice enhancement! I wanted to add similar feature when I was debugging
the TunnelCreek graphics vgabios (sometimes it got overlapped sections
in the u-boot.rom without any warnings), but never started to work on
it. Thanks for the patch.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin
Simon Glass Aug. 22, 2015, 4:19 a.m. UTC | #2 
On 19 August 2015 at 08:33, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Sun, Aug 16, 2015 at 4:37 AM, Simon Glass <sjg@chromium.org> wrote:
>> Since U-Boot and its device tree can grow we should check that it does not
>> overlap the regions above it. Track the ROM offset that U-Boot reaches and
>> check that other regions (written after U-Boot) do not interfere.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
> Nice enhancement! I wanted to add similar feature when I was debugging
> the TunnelCreek graphics vgabios (sometimes it got overlapped sections
> in the u-boot.rom without any warnings), but never started to work on
> it. Thanks for the patch.

Yes it can silently bite you, so I thought it was a good idea to fix it.

>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>
> Regards,
> Bin

Applied to u-boot-x86.
diff mbox

Patch

diff --git a/tools/ifdtool.c b/tools/ifdtool.c
index df16616..510a99c 100644
--- a/tools/ifdtool.c
+++ b/tools/ifdtool.c
@@ -706,10 +706,11 @@  int inject_region(char *image, int size, int region_type, char *region_fname)
  *			0xffffffff so use an address relative to that. For an
  *			8MB ROM the start address is 0xfff80000.
  * @write_fname:	Filename to add to the image
+ * @offset_uboot_top:	Offset of the top of U-Boot
  * @return number of bytes written if OK, -ve on error
  */
 static int write_data(char *image, int size, unsigned int addr,
-		      const char *write_fname)
+		      const char *write_fname, int offset_uboot_top)
 {
 	int write_fd, write_size;
 	int offset;
@@ -719,6 +720,14 @@  static int write_data(char *image, int size, unsigned int addr,
 		return write_fd;
 
 	offset = (uint32_t)(addr + size);
+	if (offset_uboot_top && offset_uboot_top >= offset) {
+		fprintf(stderr, "U-Boot image overlaps with region '%s'\n",
+			write_fname);
+		fprintf(stderr,
+			"U-Boot finishes at offset %x, file starts at %x\n",
+			offset_uboot_top, offset);
+		return -EXDEV;
+	}
 	debug("Writing %s to offset %#x\n", write_fname, offset);
 
 	if (offset < 0 || offset + write_size > size) {
@@ -756,24 +765,23 @@  static int write_uboot(char *image, int size, struct input_file *uboot,
 {
 	const void *blob;
 	const char *data;
-	int uboot_size;
+	int uboot_size, fdt_size;
 	uint32_t *ptr;
 	int data_size;
 	int offset;
 	int node;
-	int ret;
 
-	uboot_size = write_data(image, size, uboot->addr, uboot->fname);
+	uboot_size = write_data(image, size, uboot->addr, uboot->fname, 0);
 	if (uboot_size < 0)
 		return uboot_size;
 	fdt->addr = uboot->addr + uboot_size;
 	debug("U-Boot size %#x, FDT at %#x\n", uboot_size, fdt->addr);
-	ret = write_data(image, size, fdt->addr, fdt->fname);
-	if (ret < 0)
-		return ret;
+	fdt_size = write_data(image, size, fdt->addr, fdt->fname, 0);
+	if (fdt_size < 0)
+		return fdt_size;
+	blob = (void *)image + (uint32_t)(fdt->addr + size);
 
 	if (ucode_ptr) {
-		blob = (void *)image + (uint32_t)(fdt->addr + size);
 		debug("DTB at %lx\n", (char *)blob - image);
 		node = fdt_node_offset_by_compatible(blob, 0,
 						     "intel,microcode");
@@ -796,7 +804,7 @@  static int write_uboot(char *image, int size, struct input_file *uboot,
 		      ucode_ptr, ptr[0], ptr[1]);
 	}
 
-	return 0;
+	return ((char *)blob + fdt_size) - image;
 }
 
 static void print_version(void)
@@ -1113,12 +1121,14 @@  int main(int argc, char *argv[])
 	}
 
 	if (mode_write_descriptor)
-		ret = write_data(image, size, -size, desc_fname);
+		ret = write_data(image, size, -size, desc_fname, 0);
 
 	if (mode_inject)
 		ret = inject_region(image, size, region_type, inject_fname);
 
 	if (mode_write) {
+		int offset_uboot_top = 0;
+
 		for (wr_idx = 0; wr_idx < wr_num; wr_idx++) {
 			ifile = &input_file[wr_idx];
 			if (ifile->type == IF_fdt) {
@@ -1126,9 +1136,10 @@  int main(int argc, char *argv[])
 			} else if (ifile->type == IF_uboot) {
 				ret = write_uboot(image, size, ifile, fdt,
 						  ucode_ptr);
+				offset_uboot_top = ret;
 			} else {
 				ret = write_data(image, size, ifile->addr,
-					 ifile->fname);
+					 ifile->fname, offset_uboot_top);
 			}
 			if (ret < 0)
 				break;