[PATCH] Fix for bug in JIT exception table allocation (no test yet)

Hi, I found a bug in the code that generates exception tables, I've attached
what I think is the correct fix.

When you run out of space writing to a buffer, the buffer management code
simply stops writing at the end of the buffer. It is the responsibility of
the caller to verify that it has stayed in bounds and perform a retry with
a larger memory estimate if not. The function writing code does this, but
the exception table code following it does not. The end result is that
exception table pointers can get registered pointing to invalid data, causing
seg-faults when an exception is thrown.

I haven't implemented a test case that reproduces the problem, but I will do
so. (I've verified the problem and the fix in the scope of a much larger
system) I'm open to suggestions as to how best to test it, I'm currently
thinking of trying to create a highly contrived situation to force exception
tables to be written at the end of a buffer that won't be long enough.

ExceptionTableAllocationFix.patch (1.6 KB)

I'm actually somewhat curious at this point why it doesn't emit the tables before
deciding it's done with the function. That'd make it possible to move all of
the eh table code earlier in the method and use retryWith... instead of the loop.

-eric

Eric Christopher wrote:

>
> Hi, I found a bug in the code that generates exception tables, I've attached
> what I think is the correct fix.
>
> When you run out of space writing to a buffer, the buffer management code
> simply stops writing at the end of the buffer. It is the responsibility of
> the caller to verify that it has stayed in bounds and perform a retry with
> a larger memory estimate if not. The function writing code does this, but
> the exception table code following it does not. The end result is that
> exception table pointers can get registered pointing to invalid data, causing
> seg-faults when an exception is thrown.
>
> I haven't implemented a test case that reproduces the problem, but I will do
> so. (I've verified the problem and the fix in the scope of a much larger
> system) I'm open to suggestions as to how best to test it, I'm currently
> thinking of trying to create a highly contrived situation to force exception
> tables to be written at the end of a buffer that won't be long enough.

I'm actually somewhat curious at this point why it doesn't emit the tables before
deciding it's done with the function. That'd make it possible to move all of
the eh table code earlier in the method and use retryWith... instead of the loop.

It looks like it might be using a different slab for the function, though I'm
not quite sure why. I'll investigate further when I get back to it. Making
it part of the function's "transaction" seemed cleaner to me as well.

Hi all, I've attached a patch containing my initial bug fix and the unit test
to verify it. Can someone please review it?

Michael Muller wrote:

Eric Christopher wrote:
>
>
> >
> > Hi, I found a bug in the code that generates exception tables, I've attached
> > what I think is the correct fix.
> >
> > When you run out of space writing to a buffer, the buffer management code
> > simply stops writing at the end of the buffer. It is the responsibility of
> > the caller to verify that it has stayed in bounds and perform a retry with
> > a larger memory estimate if not. The function writing code does this, but
> > the exception table code following it does not. The end result is that
> > exception table pointers can get registered pointing to invalid data, causing
> > seg-faults when an exception is thrown.
> >
> > I haven't implemented a test case that reproduces the problem, but I will do
> > so. (I've verified the problem and the fix in the scope of a much larger
> > system) I'm open to suggestions as to how best to test it, I'm currently
> > thinking of trying to create a highly contrived situation to force exception
> > tables to be written at the end of a buffer that won't be long enough.
>
> I'm actually somewhat curious at this point why it doesn't emit the tables before
> deciding it's done with the function. That'd make it possible to move all of
> the eh table code earlier in the method and use retryWith... instead of the loop.

It looks like it might be using a different slab for the function, though I'm
not quite sure why. I'll investigate further when I get back to it. Making
it part of the function's "transaction" seemed cleaner to me as well.

The memory manager interface defines a separate set of methods for dealing
with exception table memory - presumably this is to allow for architectures
where exception tables can't be in the same pages as the functions.

ExceptionTableOverflowFix.patch (4.15 KB)