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:av1...@comtv.ru
