279273 - Broken pointer arithmetic in jsarena code

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Jürgen Kreileder]

User-Agent:       Mozilla/5.0 (X11; U; Linux ppc64; en-US; rv:1.7.5) Gecko/20050119 Galeon/1.3.19 (Debian package 1.3.19-1)
Build Identifier: Mozilla/5.0 (X11; U; Linux ppc64; en-US; rv:1.7.5) Gecko/20050119 Galeon/1.3.19 (Debian package 1.3.19-1)

The code in jsarena.h and jsarena.c does some bad pointer arithmetic.  Here's
one example:

for (a = pool->current; a->avail + nb > a->limit; pool->current = a) {

The problem with "a->avail + nb > a->limit" is that the addition might overflow
if a->avail is close to the upper memory boundary (e.g. 0xffffe184 + 8000 ==
0xc4).  The consequence is that the comparision gives an unexpected result and a
crash later on. 
The solution is to use "a->avail > a->limit - nb" instead.

I've seen crashes caused by this on Linux with an ppc64 kernel running a ppc32
mozilla build.  The problem might also be reproducible with AMD64 kernels and
32-bit mozilla builds.  The reason is that a->avail happens to be close the
upper 32-bit memory boundary in these situations.  It probably isn't easily
reproducibe on pure 32-bit and 64-bit systems.

I'm attaching a patch which fixes the problem for me.  It just touches three
instances of bad pointer arithmetic.  You might want to check if there are more.
 (Only the changes in JS_ARENA_ALLOCATE_CAST and JS_ArenaAllocate were needed to
fix the crash I saw.  I just modified JS_ARENA_GROW_CAST because it looked wrong
too.)

(I've originally reported this bug agaist Galeon but it turned out be a problem
in Mozilla. See http://bugzilla.gnome.org/show_bug.cgi?id=164262)


Reproducible: Always

Comment 1

[Jürgen Kreileder]

Attachment: JS Arena Fix

Comment 2

[Brendan Eich [:brendan]]

Attachment: revised fix

There's no need for the first change to JS_ARENA_GROW_CAST in attachment
172007 [details] [diff] [review], since the allocation at p is JS_ARENA_ALIGN(pool, size) bytes long, and
its last byte is at most at 0xffffffff.  With 32-bit jsuword type, even if p is
up against the end of the 32-bit address space, _a->avail will be zero, the
outer if's condition will be true, and the inner if/else chain will find that
the allocation cannot be grown in place.

/be

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 172299 [details] [diff] [review]
revised fix

r=shaver

Comment 4

[Brendan Eich [:brendan]]

Fixed, thanks to all.

/be