bug in 4.2BSD malloc with -DRCHECK

[John P. Linderman]

SYNOPSIS
    Realloc can cause errors when malloc is compiled with -DRCHECK -Ddebug

SYMPTOMS
    If /usr/src/lib/libc/gen/malloc.c is compiled with -DRCHECK and -Ddebug
    to do range checking, a realloc that expands an allocation, but not by
    enough to push it into a larger bucket, will cause an ASSERT error when
    the area is freed.

REPEAT BY
    Compile the following test program together with malloc.c, making sure
    that RCHECK and debug are defined when malloc.c is compiled.

    #include <stdio.h>

    main()
    {
	char line[BUFSIZ], *p, *q, *malloc(), *realloc();
	int m, n;

	p = NULL;
	while (gets(line)) {
	    switch (*line) {
	    case 'q':	/* quit */
		return (0);
	    case 'a':	/* allocate */
	    case 'r':	/* reallocate */
		n = atoi(line+1);
		if (n <= 0) {
		    (void) fprintf(stderr, "Must specify a size > 0\n");
		} else {
		    switch (*line) {
		    case 'a':
			p = malloc(n);
			break;
		    case 'r':
			p = realloc(p, n);
			break;
		    }
		    for (q = p, m = n; --m >= 0; *q++ = 0);
		    (void) printf("%d bytes at %#x\n", n, p);
		}
		break;
	    case 'f':	/* free */
		if (p != NULL) {
		    free(p);
		    p = NULL;
		}
		break;
	    default:
		(void) fprintf(stderr, "aN allocate N bytes\n");
		(void) fprintf(stderr, "f  free last allocation\n");
		(void) fprintf(stderr, "rN reallocate N bytes\n");
		(void) fprintf(stderr, "q  quit\n");
	    }
	}
	exit(0);
    }

    Run the result with input

    a400
    r480
    f
    q

    This causes an ASSERT error out of free().

FIX BY
    Change the following lines in subroutine realloc() in malloc.c

	/* avoid the copy if same size block */
	if (was_alloced &&
!	    nbytes <= onb && nbytes > (onb >> 1) - sizeof(*op) - RSLOP)
		return(cp);

    to instead be

	/* avoid the copy if same size block */
	if (was_alloced &&
!	    nbytes <= onb && nbytes > (onb >> 1) - sizeof(*op) - RSLOP) {
+   #ifdef RCHECK
+		op->ov_size = ((nbytes + sizeof(union overhead) + RSLOP) + 3 & ~3) - 1;
+		*((u_int *)((caddr_t)op + op->ov_size + 1 - RSLOP)) = RMAGIC;
+   #endif
		return(cp);
+	}
    
    so the magic number will be where free expects to find it.
    There remains a "peculiarity" when 20 or fewer bytes are reallocated
    with RCHECK defined.  Under these circumstances,
	nbytes > (onb >> 1) - sizeof(*op) - RSLOP
    fails because the right hand side of the inequality is logically
    negative but nbytes is unsigned.  Thus, space is always reallocated,
    even when existing space would have been suitable.  This can't
    happen when RCHECK is undefined, and nobody ever promised that
    realloc wouldn't do unnecessary copies, so I didn't bother to "fix" it.
    Logically speaking, the definition of RCHECK ought to force the
    definition of debug, like

    #ifdef RCHECK
    #define debug
    #endif RCHECK

    since turning on RCHECK without turning on debug simply adds to the
    overhead without reporting any errors that are detected.  Malloc is
    nowhere near my favorite piece of software, so I shall charitably
    shut up at this point.

    John P. Linderman  Department of Bad Checks  allegra!jpl