----- Original Message -----
> Hi Andrew,
>
> On 2/08/2012 7:35 PM, Andrew Hughes wrote:
> > ----- Original Message -----
> >> Andrew et al,
> >>
> >> AFAICS here:
> >>
> >> 220 encoding_variant = malloc(strlen(temp)+1);
> >> 221 if (encoding_variant == NULL) {
> >> 222 JNU_ThrowOutOfMemoryError(env, NULL);
> >> 223 return 0;
> >> 224 }
> >>
> >> we also need to do free(temp). Similarly later where we return
> >> with
> >> OOM
> >> due to realloc failure, don't we also need to free what was
> >> previously
> >> malloc'd?
> >>
> >
> > I was thinking the same just before committing, but didn't want to
> > delay
> > the main fix any further.
> >
> > While logically we do need one, I'm not sure it's worthwhile if
> > we're going
> > to throw the exception and exit anyway? Will the return even be
> > reached?
> > If we're already near enough to OOM that we can't allocate more
> > memory,
> > it's unlikely freeing temp is going to get us much, and I presume
> > it will
> > be freed anyway when the VM process exists.
>
> Are we definitely going to exit on this error? I agree if we're out
> of
> memory we're likely to continue to run into problems if we try to
> continue. But I'd prefer to see proper cleanup rather than making
> assumptions about the context in which the code is used.
>
> > But I can add it if necessary. It's trivial after all and does not
> > real
> > harm.
>
> It will also prevent us getting flagged with memory-leak
> warnings/errors
> from static analysis tools.
>
This should cover it, I think:
http://cr.openjdk.java.net/~andrew/buffer_overflow/webrev.02/
but it'll need a bug ID.
> Thanks,
> David
>
> >> David
> >>
> >> On 2/08/2012 7:18 AM, Andrew Hughes wrote:
> >>>
> >>>
> >>> ----- Original Message -----
> >>>> On 01/08/2012 14:52, Andrew Hughes wrote:
> >>>>> :
> >>>>>
> >>>>>
> >>>>> In any case, there is a Sun bug open for this:
> >>>>>
> >>>>> 6844255: Potential stack corruption in GetJavaProperties
> >>>>>
> >>>>> Can I take it that I can just get on and push Omair's extended
> >>>>> version now then,
> >>>>> with that bug ID?
> >>>> Yes, go ahead, I should have said that in my mail.
> >>>>
> >>>
> >>> Thanks.
> >>>
> >>> Done:
> >>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-August/010993.html
> >>>
> >>> with Omair as author and yourself and I as reviewers.
> >>>
> >>>>> Well, the locale can be set be an environment variable, so it
> >>>>> could
> >>>>> potentially
> >>>>> be anything of any length...
> >>>>>
> >>>>> The Debian bug posted above has an example, though I couldn't
> >>>>> replicate it.
> >>>>>
> >>>> I couldn't replicate it either and was just curious if anyone
> >>>> managed
> >>>> to
> >>>> demonstrate it.
> >>>>
> >>>
> >>> Yeah, I tend to think it's more potentially exploitable rather
> >>> than
> >>> something
> >>> that's actually been hit.
> >>>
> >>>> -Alan.
> >>>>
> >>>
> >>> Thanks,
> >>
> >
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
