[9fans] kernel possible double free

[9fans] kernel possible double free

[Yoann Padioleau]

Hi,

I think I've found a possible situation where we call two times free on the same pointer.
in sysexec() there is essentially

sysexec(...) {
 … 
	if(waserror()){
		free(file0);
		free(elem);
		nexterror();
	}

	for(;;){
		tc = namec(file, Aopen, OEXEC, 0);
		if(waserror()){
			cclose(tc);
			nexterror();
		}

        …
       }
	qlock(&up->seglock);
	if(waserror()){
		qunlock(&up->seglock);
		nexterror();
	}

     …
	free(file0);
+      file0 = nil; <------------------------- we should add that, for the same reason we do elem = nil below
	free(up->text);
	up->text = elem;
	elem = nil;	/* so waserror() won't free elem */
	USED(elem);

    …
	qunlock(&up->seglock);
	poperror();	/* seglock */
-	poperror();	/* elem */ <----------------------- actually this is not the poperror of elem, but of tc

        …
	poperror();
	cclose(tc);
+      poperror(); /* elem and file0 */ <----------- this is where the poperror of elem should be.


}

Re: [9fans] kernel possible double free

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

On 9 June 2014 08:40, Yoann Padioleau <pad@fb.com> wrote:

> I think I've found a possible situation where we call two times free on
> the same pointer.
> in sysexec() there is essentially
>

the only correct way to write these is not to rely on nil values or not,
but immediately after the allocation, include a waserror, and then poperror
at the appropriate point when done with the value.
unless values have exactly the same life time, they should not be freed in
the same waserror block.
exec has been one of the trickier cases historically.

[-- Attachment #2: Type: text/html, Size: 952 bytes --]