MOO-cows Mailing List Archive

[Subject Prev][Subject Next][Thread Prev][Thread Next][Subject Index][Thread Index]

[nop@kagoona.mitre.org: Catching E_PROPNF leaks in p5/p6/r2]



------- Start of forwarded message -------
Return-Path: nop@ccs.neu.edu
Date: Fri, 23 May 1997 14:32:30 -0400 (EDT)
From: "Jay A. Carlson" <nop@kagoona.mitre.org>
To: moo-cows@the-b.org
Subject: Catching E_PROPNF leaks in p5/p6/r2
Sender: owner-moo-cows@the-b.org
Precedence: bulk
Resent-From: clue-cows <nop@nop.com>
Errors-To: clue-cows <nop@nop.com>

Ben suggested that after not finding any more easy leaks in my work on
1.8.0p6 running jhcore, I should try running lambdacore instead.
Bingo.

Summary: in +d verbs, referring to properties that don't
exist does not free the reference to the property name.

In the past, we probably would never have noticed this, since the big
hairy traceback tended to discourage repeat offenders.  However, the
now common `foo.bar ! E_PROPNF' idiom means this happens a lot.  

Not freeing the reference to "bar" isn't a problem, since the calling
verb still holds a reference to it anyway; it becomes a leak when the
verb is rewritten.  But foo.(baz) is a problem---and foo.(msgname +
"_msg") is guaranteed to leak.

The bug in execute.c happens because PUSH_ERROR()'s macro expansion
may return directly from run().  It's a shame C doesn't have try/finally.

You know, when it's very late at night (and I've had far too many CNS
stimulants,) paranoia starts to set in.  Maybe these bugs are
carefully placed here as barriers for those who would dare try to hack
the server.  Or maybe as trials, to test our faith---when we find
them, the spell will be broken and we'll wake.  Pavel will say to us: "The
illusion was necessary.  I would never really leave you."  As he
cocks the shotgun, th

Oh, you mean you just want the patch?  OK:

diff -c -r1.6 -r1.6.2.1
*** execute.c	1997/03/08 06:25:39	1.6
- --- execute.c	1997/05/23 07:03:44	1.6.2.1
***************
*** 1353,1370 ****
  		    db_prop_handle h;
  
  		    h = db_find_property(obj.v.obj, propname.v.str, &prop);
! 		    if (!h.ptr)
  			PUSH_ERROR(E_PROPNF);
! 		    else if (h.built_in
  			 ? bi_prop_protected(h.built_in, RUN_ACTIV.progr)
! 		      : !db_property_allows(h, RUN_ACTIV.progr, PF_READ))
  			PUSH_ERROR(E_PERM);
! 		    else if (h.built_in)
  			PUSH(prop);	/* it's already freshly allocated */
! 		    else
  			PUSH_REF(prop);
! 		    free_var(propname);
! 		    free_var(obj);
  		}
  	    }
  	    break;
- --- 1353,1378 ----
  		    db_prop_handle h;
  
  		    h = db_find_property(obj.v.obj, propname.v.str, &prop);
! 		    if (!h.ptr) {
! 			free_var(propname);
! 			free_var(obj);
  			PUSH_ERROR(E_PROPNF);
! 		    } else if (h.built_in
  			 ? bi_prop_protected(h.built_in, RUN_ACTIV.progr)
! 		    : !db_property_allows(h, RUN_ACTIV.progr, PF_READ)) {
! 			free_var(propname);
! 			free_var(obj);
  			PUSH_ERROR(E_PERM);
! 		    } else if (h.built_in) {
! 			free_var(propname);
! 			free_var(obj);
! 
  			PUSH(prop);	/* it's already freshly allocated */
! 		    } else {
! 			free_var(propname);
! 			free_var(obj);
  			PUSH_REF(prop);
! 		    }
  		}
  	    }
  	    break;
***************
*** 1480,1492 ****
  
  		    if (err == E_NONE) {
  			db_set_property_value(h, var_ref(rhs));
  			PUSH(rhs);
  		    } else {
  			free_var(rhs);
  			PUSH_ERROR(err);
  		    }
- - 		    free_var(propname);
- - 		    free_var(obj);
  		}
  	    }
  	    break;
- --- 1488,1502 ----
  
  		    if (err == E_NONE) {
  			db_set_property_value(h, var_ref(rhs));
+ 			free_var(propname);
+ 			free_var(obj);
  			PUSH(rhs);
  		    } else {
  			free_var(rhs);
+ 			free_var(propname);
+ 			free_var(obj);
  			PUSH_ERROR(err);
  		    }
  		}
  	    }
  	    break;

Jay Carlson   nop@kagoona.mitre.org    nop@nop.com
The MITRE Corporation, Bedford MA

Flat text is just *never* what you want.       ---stephen p spackman
------- End of forwarded message -------

Home | Subject Index | Thread Index