[coreboot] [PATCH][RFC] LAR segfault when adding to a full archive

[Carl-Daniel Hailfinger]

Hi,

Alvar Kusma found a bug in util/lar: If you try to add a file to a full
LAR archive, the LAR utility will segfault. This is reproduced easily by
zerofilling the LAR, then adding anything to it.

Looking at the code, the reason is obvious:
lar_empty_offset() can return an error code (-1). None of the callers
check for an error code, they simply assume the return value is valid.

Preliminary patch follows (and raises a few questions).
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: util/lar/lar.h
===================================================================
--- util/lar/lar.h	(Revision 646)
+++ util/lar/lar.h	(Arbeitskopie)
@@ -58,7 +58,9 @@
 #define BOOTBLOCK_NAME_LEN 16
 
 typedef uint64_t u64;
+typedef int64_t s64;
 typedef uint32_t u32;
+typedef int32_t s32;
 typedef uint8_t  u8;
 
 /* NOTE -- This and the coreboot lar.h may NOT be in sync. Be careful. */
Index: util/lar/stream.c
===================================================================
--- util/lar/stream.c	(Revision 646)
+++ util/lar/stream.c	(Arbeitskopie)
@@ -492,7 +492,7 @@
  * @param lar the LAR archive structure
  * @return The offset of the first chunk of empty space
  */
-static int lar_empty_offset(struct lar *lar)
+static s64 lar_empty_offset(struct lar *lar)
 {
 	u32 offset = 0;
 	struct lar_header *header;
@@ -508,10 +508,12 @@
 		offset += get_next_offset(header);
 	}
 
-	if (offset >= get_bootblock_offset(lar->size))
+	if (offset >= get_bootblock_offset(lar->size)) {
+		err("No empty space found!\n");	
 		return -1;
+	}
 
-	return offset;
+	return (s64)offset;
 }
 
 /**
@@ -825,11 +827,16 @@
 int maxsize(struct lar *lar, char *name)
 {
 	int size;
-	u32 offset;
+	s64 offset;
 	int bootblock_size;
 
 	/* Find the beginning of the available space in the LAR */
+#warning We should check all chunks of free space in the LAR. Right now we do not return the maximum size, but the size of the first chunk.
 	offset = lar_empty_offset(lar);
+	if (offset < 0) {
+		err("maxsize is negative\n");
+		return offset;
+	}
 
 	/* Figure out how big our header will be */
 	size = get_bootblock_offset(lar->size) - offset - header_len(name,NULL) - 1;
@@ -878,7 +885,7 @@
 	int ret, hlen;
 	int pathlen;
 	u32 *walk,  csum;
-	u32 offset;
+	s64 offset;
 
 	/* Find the beginning of the available space in the LAR */
 	offset = lar_empty_offset(lar);
@@ -886,7 +893,8 @@
 	/* Figure out how big our header will be */
 	hlen = header_len(pathname, &pathlen);
 
-	if (offset + hlen + complen >= get_bootblock_offset(lar->size)) {
+	if ((offset < 0) || 
+		(offset + hlen + complen >= get_bootblock_offset(lar->size))) {
 		err("Not enough room in the LAR to add the file.\n");
 		return -1;
 	}