[coreboot] [PATCH][RFC] LAR segfault when adding to a full archive
Stefan Reinauer
stepan at coresystems.de
Fri Mar 28 10:36:05 CET 2008
NACK as long as the code adds #warnings. Please fix the issues and resubmit.
Carl-Daniel Hailfinger wrote:
> 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;
> }
>
>
>
>
More information about the coreboot
mailing list