Re: [Qemu-devel] [Qemu-arm] getdents patch for 64-bit app on 32-bit host

[Peter Maydell]

On 17 April 2018 at 22:53, Henry Wertz <address@hidden> wrote:
> Peter Maydell has raised a concern about possible buffer overflows in this
> code (which was meant to handle 32-bit app on 64-bit system, not 64-bit on
> 32-bit).  I must admit I haven't gone through the dirent-copying code with a
> fine-toothed comb... it appeared to work for my use case.  That said, the
> code seems to be careful about using offsetof() rather than making any
> assumptions.  In addition, the dirent-copying code appears to have an assert
> that would crash qemu if it was going to write past the end of the dirent
> buffer -- always nice to have plenty of sanity checks!

If you build the attached test program for x86-64 (which is a
minor tweak on the test program in the Linux getdents manpage):
 gcc -g -Wall -o /tmp/getdents getdents.c -static

and then on a 32-bit Arm host take a qemu-x86_64 with your patch
applied, and a test directory like this:

$ ls /tmp/testdir/
abcd  abcde

and run it, QEMU will abort on the assert that we don't run off
the end of the buffer:

$ ./build/all-a32/x86_64-linux-user/qemu-x86_64 ~/getdents /tmp/testdir
linux_dirent struct size 24 bytes
buffer space 32 bytes
qemu-x86_64: /home/peter.maydell/qemu/linux-user/syscall.c:10197:
do_syscall: Assertion `count1 + treclen <= count' failed.

This is because the guest linux_dirent is bigger than the host
linux_dirent, and therefore just because the host syscall
successfully fit the record into the buffer doesn't mean we
can fit the guest record into the buffer.

I don't see any way to fix this, because the records are variable size.

thanks
-- PMM

getdents.c
Description: Text Data