> On Sunday 03 September 2006 18:02, Jeroen Frijters wrote:
> > ...
> > Most people do not understand the (security) implications of using
> > native code in combination with handles and most of the code out there
> > is simply wrong (as is the current patch). See [1].
>
> i'll have to read this document first.
>
> > Regards,
> > Jeroen
> >
> > [1]
> > http://www.hpl.hp.com/personal/Hans_Boehm/misc_slides/java_finalizers.pd
> > f

and later,
On Sunday 03 September 2006 22:29, Jeroen Frijters wrote:
>
> The finalizer looks dangerous. For example, a subclass could call
> finalize multiple times, resulting in multiple calls to free() (which
> may or may not be exploitable). Note that you cannot solve this by only
> fixing the finalizer (see the PDF I linked to previously).

i had a look at the slides and i wrote a small Mauve test (attached) to test 
some changes to the code of both the finalize() and natFinalize() methods.  
unfortunately i was not able to test it --the VM (cacao) throws a SIGSEV on 
the second finalize.

i followed the advice given in [1] and synchronized the code in the BigInteger 
finalize() method which now looks like so:

  protected void finalize() throws Throwable
  {
    try
    {
      synchronized (this)
      {
        if (USING_NATIVE)
          this.natFinalize();
      }
    }
    finally
    {
      super.finalize();
    }
  }

in addition, the natFinalize() code now looks like so:

  _this = (mpz_ptr)(*env)->GetObjectField (env, this, native_ptr);
  if (_this != NULL)
    {
      mpz_clear (_this);
      free (_this);
      _this = NULL;
    }

those changes should address your concern about calling free() multiple times.


a future version of the native code, could follow more closely the other 
suggestions in [1] and also would allow less fragmentation of memory by, for 
example:

a. allocating a pool of data structure from which needed mpz_t for BigIntegers 
and work variables can be pulled.

b. when a natInitialize() method or a work mpz_t is needed, an entry in the 
pool can be marked as in-use.

c. replace the natFinalize() with returning the associated mpz_t to the pool.

d. add an implementation of the JNI 1.2 On_Unload method which frees the pool.


cheers;
rsn
/* finalize.java
   Copyright (C) 2006 Free Software Foundation, Inc.
This file is part of Mauve.

Mauve is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2, or (at your option)
any later version.

Mauve is distributed in the hope that it will be useful, but
WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
General Public License for more details.

You should have received a copy of the GNU General Public License
along with Mauve; see the file COPYING.  If not, write to the
Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA.

*/

// Tags: JDK1.2

package gnu.testlet.java.math.BigInteger;

import gnu.testlet.TestHarness;
import gnu.testlet.Testlet;

import java.math.BigInteger;

/**
 * tests for the BigInteger.finalize() method.
 */
public class finalize
    implements Testlet
{
  public void test(TestHarness harness)
  {
    harness.checkPoint("finalize");
    try
    {
      BadBigInteger x = new BadBigInteger("104");
      x.finalize();
      x.finalize();
    }
  catch (Throwable x)
    {
      harness.debug(x);
      harness.fail("finalize: " + x);
    }
  }
  
  class BadBigInteger extends BigInteger
  {
    BadBigInteger(String s)
    {
      super(s);
    }

    public void finalize() throws Throwable
    {
      super.finalize();
    }
  }
}

Attachment: pgpLVRTD97GIK.pgp
Description: PGP signature

Reply via email to