[comp.unix.ultrix] Bug in 'brk'

mcm@rti.UUCP (Mike Mitchell) (07/31/89)

I have run across a bug with Ultrix 3.1 on both the DecStation 3100 and
the bigger vaxes.  It involves using 'brk()' to allocate and free memory.
The problem is that a process' PTE's are not invalidated properly when
freeing memory.  That means that a program can access memory it has just
freed.  It does not show up on microvaxes because their TLB cache is so
small.  The DecStation 3100 has a 64-entry TLB, so it does have the bug.
The bug also shows up on 8600's and 785's, so you might want to test
your own system.  Here is a program that shows off the bug:

----------------------------------------------------------------------------
#include <signal.h>

main()
{
    char *old_break, *cp;
    int i;
    extern char *sbrk(), *brk();
    void segv();

    signal(SIGSEGV, segv);

    i = getpagesize();
    old_break = sbrk(0);		/* get the current "break" */
    (void) brk(old_break + 2*i);	/* bump it up 2 pages */

    cp = old_break + i + 256;
    *cp = 1;				/* write into a new page */

    (void) brk(old_break);		/* return the memory */

    *cp = 2;				/* write into the page again.  This */
					/* time, you should get a sigsegv */

    printf("Your brk routine is broken!\n");
    exit(1);
}

void segv()
{
    printf("Your brk routine works correctly.\n");
    exit(0);
}
----------------------------------------------------------------------------

I have verified that the bug is present in BSD 4.2 and BSD 4.3, but I know
it has been fixed in BSD 4.3 tahoe.  I have seen this bug reported in other
newsgroups several times!  A fix for the bug has been known for several years,
yet few vendors have incorporated the fix.

The problem is in the 'vm_proc.c' file, in the routine
'expand()'.  The starting address for the PTE's to invalidate is not
calculated correctly when freeing memory.  The code in error looks something
like:

	if (change < 0)
	    change = -change;
	else {

The code should read:

	if (change < 0) {
	    change = -change;
	    v -= change;
	} else {

Further down in the code 'v' is passed on to 'newptes()', and it sets up the
PTE's.
-- 
Mike Mitchell	{decvax,seismo,ihnp4,philabs}!mcnc!rti!mcm  mcm@rti.rti.org

"If you hear me talking on the wind, You've got
 to understand, We must remain perfect strangers"	    (919) 541-6098