Re: [Qemu-devel] [security bug]code_gen_buffer can be overflowed

[Blue Swirl]

On 12/1/07, Blue Swirl <address@hidden> wrote:
> On 12/1/07, TeLeMan <address@hidden> wrote:
> >
> >
> > Blue Swirl-2 wrote:
> > >
> > > On 11/28/07, TeLeMan <address@hidden> wrote:
> > >>
> > >> dyngen_code() can generate more than CODE_GEN_MAX_SIZE bytes,
> > >> code_gen_buffer
> > >> can be overflowed. I hope this security bug will be fixed soon.
> > >
> > > Thank you for the analysis. It's true that cpu_gen_code does not pass
> > > CODE_GEN_MAX_SIZE (65536) on to gen_intermediate_code and that should
> > > be fixed. But gen_intermediate_code can only add OPC_MAX_SIZE (512 -
> > > 32) instructions more, so there is no security bug.
> > >
> > >
> >
> > This POC is a windows exe and was tested on QEMU v0.9.0 (Guest OS is Windows
> > XP SP2).
> > This overflow will overwrite the TranslationBlock buffer.
> >
> > http://www.nabble.com/file/p14101223/qemu-dos.rar qemu-dos.rar
>
> I see my error, gen_intermediate_code produces ops, not host
> instructions. For each op several host instructions are generated, for
> Sparc32 maximum on my machine is 170 but for ARM this can be 840. In
> the worst case, (512 - 32) * 840 = 403200 bytes are generated, thus a
> buffer overflow is indeed possible.
>
> I can see a few possible fixes for this.
>
> The buffer size can be increased from 64k to 512k or the buffer can be
> allocated dynamically after calculating the maximum instruction size.
>
> OPC_BUF_SIZE can be decreased from 512 to 50.
>
> All ops can be made smaller by introducing more helpers.
>
> dyngen_code loop could check for buffer size.

Actually the buffer size is OK, but the safety margin was not large
enough. In this patch the margin is calculated from maximum block
size.

GCC could calculate the maximum on compile time, but it doesn't, so
the code is not optimal. Any suggestions for more advanced CPP magic
to calculate the maximum of a list of constants?

The patch works for Sparc target on x86_64 host. I didn't test other
combinations, so especially ARM target on RISC hosts with larger
generated code (ia64?) and/or smaller CODE_GEN_BUFFER_SIZE (alpha)
should be checked. The maximum should not exceed the buffer size or no
code can be generated. In that case, also OPC_BUF_SIZE should be
decreased.

Because of the security aspects, I think it's better to commit this
pretty soon and not wait for the confirmation for all host/target
combinations. If some combination happens to break, it can be fixed
quickly.

fix_code_gen_of.diff
Description: Text Data