[PATCH] [libpayload] fix memory allocator free list corruption

[Stefan Reinauer]

See patch

fix memory allocator free list corruption

[Jordan Crouse]

On 14/08/08 13:12 +0200, Stefan Reinauer wrote:
> See patch
> fix memory allocator bug that lead to freelist corruption on the first malloc
> (and spent 8 bytes too much per malloc)
> if the memory allocator detects freelist corruption, print a message instead
> of silently dying.
> Signed-off-by: Stefan Reinauer

Acked-by: Jordan Crouse

I'm not 100% sure how we want to handle fatal errors.  I am not convinced
that a printf() -> halt() is the most customer friendly way.  But we have
so few of them right now, we don't need a solution right away.

Jordan
> Index: libc/malloc.c
>
> --- libc/malloc.c	(revision 3509)
> +++ libc/malloc.c	(working copy)
> @@ -92,13 +92,15 @@
>    	header = *((hdrtype_t *) ptr);
>    	int size = SIZE(header);
> if (!HAS_MAGIC(header) || size == 0)
> if (!HAS_MAGIC(header) || size == 0) {
> 	printf("memory allocator panic.\n");
> halt();
> }
>
> if (header & FLAG_FREE) {
> 	if (len <= size) {
> 		void *nptr = ptr + (HDRSIZE + len);
> 		int nsize = size - (len + 8);
> 		int nsize = size - (HDRSIZE + len);
>
> 	/* Mark the block as used. */
> 	*((hdrtype_t *) ptr) = USED_BLOCK(len);
>
> @@ -109,7 +111,7 @@
> 		if (nsize > 0)
> 			*((hdrtype_t *) nptr) =
> 			    FREE_BLOCK(nsize - 4);
> 			    FREE_BLOCK(nsize);
>
> 	return (void *)(ptr + HDRSIZE);
> }
>

fix memory allocator free list corruption

[Stefan Reinauer]

Jordan Crouse wrote:
> > fix memory allocator bug that lead to freelist corruption on the first malloc
> > (and spent 8 bytes too much per malloc)
> > if the memory allocator detects freelist corruption, print a message instead
> > of silently dying.
> > Signed-off-by: Stefan Reinauer
>
> Acked-by: Jordan Crouse

thanks! r3510.
> I'm not 100% sure how we want to handle fatal errors.  I am not convinced
> that a printf() -> halt() is the most customer friendly way.  But we have
> so few of them right now, we don't need a solution right away.

Yes, I thought about that, too. I introduced the printf merely to
distinguish the fatal cases caught by the code from those overwriting
the code or hanging the machine..

We could have a fatal() function that calls into a handler if the
payload installs one, or prints a warning and halts otherwise. I have no
strong opinion about this, just giving the user or developer a chance to
recognize what went wrong can be useful. In practice, those problems
won't happen anyways unless the payload (or, libpayload) has a bug.

Stefan