[Qemu-devel] linux-user issues

[Qemu-devel] linux-user issues

[malc]

a) elfload.c:859

   #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned long)(TARGET_ELF_EXEC_PAGESIZE-1))

   This means that for 64bit guest on a 32bit host the _v's value is
   silently reduced to 32bit, the cast should be abi_ulong.

b) mmap.c:428

   real_start = start & qemu_host_page_mask;

   Same thing basically qemu_host_page_mask is unsigned long and so
   the upper bits are sliently cleared. Again qemu_host_page_mask
   should probably be abi_ulong.

The above two make elf binary with one of the segments above 4G load
and run on 32bit guest only to fail well into execution and without
any indication that it shouldn't have been allowed to run in the first
place.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: linux-user issues

[Paul Brook]

> a) elfload.c:859
> 
>    #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
>  long)(TARGET_ELF_EXEC_PAGESIZE-1))
> 
>    This means that for 64bit guest on a 32bit host the _v's value is
>    silently reduced to 32bit, the cast should be abi_ulong.
> 
> b) mmap.c:428
> 
>    real_start = start & qemu_host_page_mask;
> 
>    Same thing basically qemu_host_page_mask is unsigned long and so
>    the upper bits are sliently cleared. Again qemu_host_page_mask
>    should probably be abi_ulong.
> 
> The above two make elf binary with one of the segments above 4G load
> and run on 32bit guest only to fail well into execution and without
> any indication that it shouldn't have been allowed to run in the first
> place.

I'd be amazed if these are the only two issues, and expect 64-bit guests on 
32-bit hosts to be generally unsafe.

Paul

[Qemu-devel] Re: linux-user issues

[malc]

On Sat, 13 Mar 2010, Paul Brook wrote:

> > a) elfload.c:859
> > 
> >    #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
> >  long)(TARGET_ELF_EXEC_PAGESIZE-1))
> > 
> >    This means that for 64bit guest on a 32bit host the _v's value is
> >    silently reduced to 32bit, the cast should be abi_ulong.
> > 
> > b) mmap.c:428
> > 
> >    real_start = start & qemu_host_page_mask;
> > 
> >    Same thing basically qemu_host_page_mask is unsigned long and so
> >    the upper bits are sliently cleared. Again qemu_host_page_mask
> >    should probably be abi_ulong.
> > 
> > The above two make elf binary with one of the segments above 4G load
> > and run on 32bit guest only to fail well into execution and without
> > any indication that it shouldn't have been allowed to run in the first
> > place.
> 
> I'd be amazed if these are the only two issues, and expect 64-bit guests on 
> 32-bit hosts to be generally unsafe.

These are the two issues i've been bitten by while trying to solve
x86_64's far jumps, if you are okay with code silently misbehaving
fine by me.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: linux-user issues

[Paul Brook]

> On Sat, 13 Mar 2010, Paul Brook wrote:
> > > a) elfload.c:859
> > >
> > >    #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
> > >  long)(TARGET_ELF_EXEC_PAGESIZE-1))
> > >
> > >    This means that for 64bit guest on a 32bit host the _v's value is
> > >    silently reduced to 32bit, the cast should be abi_ulong.
> > >
> > > b) mmap.c:428
> > >
> > >    real_start = start & qemu_host_page_mask;
> > >
> > >    Same thing basically qemu_host_page_mask is unsigned long and so
> > >    the upper bits are sliently cleared. Again qemu_host_page_mask
> > >    should probably be abi_ulong.
> > >
> > > The above two make elf binary with one of the segments above 4G load
> > > and run on 32bit guest only to fail well into execution and without
> > > any indication that it shouldn't have been allowed to run in the first
> > > place.
> >
> > I'd be amazed if these are the only two issues, and expect 64-bit guests
> > on 32-bit hosts to be generally unsafe.
> 
> These are the two issues i've been bitten by while trying to solve
> x86_64's far jumps, if you are okay with code silently misbehaving
> fine by me.

I never said this was ok behavior. Only that you've probably only discovered 
the tip of the iceberg.

Paul

[Qemu-devel] Re: linux-user issues

[malc]

On Sun, 14 Mar 2010, Paul Brook wrote:

> > On Sat, 13 Mar 2010, Paul Brook wrote:
> > > > a) elfload.c:859
> > > >
> > > >    #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
> > > >  long)(TARGET_ELF_EXEC_PAGESIZE-1))
> > > >
> > > >    This means that for 64bit guest on a 32bit host the _v's value is
> > > >    silently reduced to 32bit, the cast should be abi_ulong.
> > > >
> > > > b) mmap.c:428
> > > >
> > > >    real_start = start & qemu_host_page_mask;
> > > >
> > > >    Same thing basically qemu_host_page_mask is unsigned long and so
> > > >    the upper bits are sliently cleared. Again qemu_host_page_mask
> > > >    should probably be abi_ulong.
> > > >
> > > > The above two make elf binary with one of the segments above 4G load
> > > > and run on 32bit guest only to fail well into execution and without
> > > > any indication that it shouldn't have been allowed to run in the first
> > > > place.
> > >
> > > I'd be amazed if these are the only two issues, and expect 64-bit guests
> > > on 32-bit hosts to be generally unsafe.
> > 
> > These are the two issues i've been bitten by while trying to solve
> > x86_64's far jumps, if you are okay with code silently misbehaving
> > fine by me.
> 
> I never said this was ok behavior. Only that you've probably only discovered 
> the tip of the iceberg.

Even on this path (mmap one) it's indeed not, further bellow there are
yet more truncations due to improperly sized and's. Furthermore the
abi_ulong sized `start' argument is passed to mmap without ever
checking if it fits 32bit, question is what to do about it.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Re: linux-user issues

[malc]

On Sun, 14 Mar 2010, malc wrote:

> On Sun, 14 Mar 2010, Paul Brook wrote:
> 
> > > On Sat, 13 Mar 2010, Paul Brook wrote:
> > > > > a) elfload.c:859
> > > > >
> > > > >    #define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
> > > > >  long)(TARGET_ELF_EXEC_PAGESIZE-1))
> > > > >
> > > > >    This means that for 64bit guest on a 32bit host the _v's value is
> > > > >    silently reduced to 32bit, the cast should be abi_ulong.
> > > > >
> > > > > b) mmap.c:428
> > > > >
> > > > >    real_start = start & qemu_host_page_mask;
> > > > >
> > > > >    Same thing basically qemu_host_page_mask is unsigned long and so
> > > > >    the upper bits are sliently cleared. Again qemu_host_page_mask
> > > > >    should probably be abi_ulong.
> > > > >
> > > > > The above two make elf binary with one of the segments above 4G load
> > > > > and run on 32bit guest only to fail well into execution and without
> > > > > any indication that it shouldn't have been allowed to run in the first
> > > > > place.
> > > >
> > > > I'd be amazed if these are the only two issues, and expect 64-bit guests
> > > > on 32-bit hosts to be generally unsafe.
> > > 
> > > These are the two issues i've been bitten by while trying to solve
> > > x86_64's far jumps, if you are okay with code silently misbehaving
> > > fine by me.
> > 
> > I never said this was ok behavior. Only that you've probably only discovered 
> > the tip of the iceberg.
> 
> Even on this path (mmap one) it's indeed not, further bellow there are
> yet more truncations due to improperly sized and's. Furthermore the
> abi_ulong sized `start' argument is passed to mmap without ever
> checking if it fits 32bit, question is what to do about it.

How about following? In the future munmap and mprotect needs to be
addressed similarly.

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 71ed2d6..25064dc 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -856,7 +856,7 @@ struct exec
 
 /* Necessary parameters */
 #define TARGET_ELF_EXEC_PAGESIZE TARGET_PAGE_SIZE
-#define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned long)(TARGET_ELF_EXEC_PAGESIZE-1))
+#define TARGET_ELF_PAGESTART(_v) ((_v) & ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1))
 #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1))
 
 #define INTERPRETER_NONE 0
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index ad00b6f..c436709 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -417,6 +417,11 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     }
 #endif
 
+    if (start > ULONG_MAX) {
+        errno = EINVAL;
+        goto fail;
+    }
+
     if (offset & ~TARGET_PAGE_MASK) {
         errno = EINVAL;
         goto fail;



-- 
mailto:av1474@comtv.ru