[Bug 12783] New: Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

Summary: Allocation of PEB and TEB can overlap dll mappings
           Product: Wine
           Version: 0.9.60
          Platform: PC
        OS/Version: Solaris
            Status: UNCONFIRMED
          Severity: critical
          Priority: P2
         Component: ntdll
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

thread_init in ntdll/thread.c is responsible for setting up the control
structures for a new thread. It allocates memory for a user data region, a PEB
(Process Environment Block), and a TEB (Thread Environment Block).

On Windows NT, the PEB and the first TEB are fixed at 0x7ffdf000 and
0x7ffde000, respectively, unless the kernel is patched for address
randomisation. The TEB is one page (see wine's definition of TEB) and adjacent
to the PEB.

thread_init uses NtAllocateVirtualMemory backed by anonymous mmaps. It
allocates a 64k region for user data at a fixed address of 0x7ffe0000. For the
PEB and TEB allocations, the system is free to choose the address. The TEB in
Wine is actually allocated as part of larger region also storing pthreads data,
so it doesn't fit in the address that Windows uses for it without overlapping
the traditional PEB region.

Later on, __wine_process_init in dlls/ntdll/loader.c loads builtin dlls to
their respective addresses. This calls NtAllocateVirtualMemory and there is
code in  
create_view to deal with overlapping segments. It is asserted that no
allocation should overlap with a segment that doesn't have the 
VFLAG_SYSTEM flag set.

In Wine 0.9.60 on Solaris, one of the dll allocations (ntdll at
7ff90000-7ffdb000
) would overlap with the PEB at 7ffb0000-7ffb1000, which triggers the assert.

Regression testing shows that this commit caused the regression, possibly
because of size changes for ntdll:
[4c6d2b01cc842ac13af6257a0c41f67ae49c992a] ntdll: Don't create a temporary dir,
run wineprefixcreate directly in the final dir.

Possible solutions:
Mapping the TEB and PEB at fixed addresses
Ensuring NtAllocateVirtualMemory mmaps do not overlap with the dll address
space accidentally.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #1 from Albert Lee   2008-04-25 02:22:25 ---
Created an attachment (id=12453)
 --> (http://bugs.winehq.org/attachment.cgi?id=12453)
WINEDEBUG=virtual output showing assert

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #2 from Albert Lee   2008-04-25 02:22:59 ---
Created an attachment (id=12454)
 --> (http://bugs.winehq.org/attachment.cgi?id=12454)
Stack trace from the assert

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #3 from Alexandre Julliard   2008-04-25 03:43:16 ---
Probably the address range we computed for ntdll is incorrect, it wouldn't make
sense to have an allocation in the middle of it. Can you check where
ntdll.dll.so is actually mapped?

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #4 from Albert Lee   2008-04-25 11:28:57 ---
(In reply to comment #3)
> Probably the address range we computed for ntdll is incorrect, it wouldn't make
> sense to have an allocation in the middle of it. Can you check where
> ntdll.dll.so is actually mapped?

If I work around the allocations, I see:
0009: load_dll( handle=0, base=7ff90000, size=307200, dbg_offset=0, dbg_size=0, 
name=7fcc0160, filename=L"ntdll.dll" )
Adding a trace to load_builtin_callback also confirms this.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #5 from Albert Lee   2008-04-25 13:07:07 ---
Created an attachment (id=12470)
 --> (http://bugs.winehq.org/attachment.cgi?id=12470)
Full WINEDEBUG trace, with process address space (library mappings) at bottom

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #6 from Albert Lee   2008-04-28 21:54:42 ---
I've solved the problem using a suggestion by Rod Evans, which is relaxing the
alignment of the data section of Wine  DLLs from 0x10000 (specified in the ABI)
to 0x1000. That way, no "hole" is created  which the OS can use for anonymous
mmaps.

http://opensolaris.org/jive/message.jspa?messageID=229817#229799

As suggested, I created a mapfile which was passed to the linker. This appears
to have fixed the problem, although I'm experiencing a crash in winex11 because
of the new XIM code.

The SFEwine package has been updated with this fix. I think a configure check
would make this more transparent for others, what do you think?

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

John Reiser  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #7 from John Reiser   2008-05-18 22:16:12 ---
There is a related underlying problem: the return value of
NtAllocateVirtualMemory() is not checked in 3 places in thread_init()
(dlls/ntdll/thread.c)  If you have a kernel that does not honor the address
given in mmap(addr,...) when !MAP_FIXED [such a kernel is entirely legal; if
!MAP_FIXED then the addr is only a hint] then the return value can be
STATUS_CONFLICTING_ADDRESSES, with no allocation at the requested addr, and NO
UPDATE of addr.  Probably wine will SIGSEGV later.  Not checking the return
value has reduced unnecessarily the precision of error messages, which
lengthens the time to debug and fix the problem.

The declaration of subroutine NtAllocateVirtualMemory ought to be marked with
"__attribute__ ((warn_unused_result))".  Three are 2 places in relay.c, plus 1
in directory.c and 1 in loader.c that also ignore the return value.

The particular case
    addr = (void *)0x7ffe0000;
    size = 0x10000;
    NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size,
MEM_RESERVE|MEM_COMMIT, PAGE_READWRITE );
    user_shared_data = addr;
probably should have some way to say MAP_FIXED, but I cannot find such a way. 
[Hint: valgrind presents such a kernel that ignores the first argument to
mmap() unless MAP_FIXED.]

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #8 from Vitaliy Margolen   2008-05-23 13:44:11 ---
(In reply to comment #7)
> The particular case
>     addr = (void *)0x7ffe0000;
>     size = 0x10000;
>     NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size,
> MEM_RESERVE|MEM_COMMIT, PAGE_READWRITE );
>     user_shared_data = addr;
> probably should have some way to say MAP_FIXED, but I cannot find such a way. 
> [Hint: valgrind presents such a kernel that ignores the first argument to
> mmap() unless MAP_FIXED.]

There is no reason to check for this particular allocation unless to die
immediately if it fails. It's the special data structure [Shared User Data]
present on all NT+ windows at the constant address. If your kernel can not
honor this, then it's broken and have to be fixed. Or listed as incompatible
with Wine. This area is critical to most copy-protection systems. Also lots of
programs make a use of this data structure.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #9 from Albert Lee   2008-06-10 17:28:46 ---
(In reply to comment #8)
> (In reply to comment #7)
> > The particular case
> >     addr = (void *)0x7ffe0000;
> >     size = 0x10000;
> >     NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size,
> > MEM_RESERVE|MEM_COMMIT, PAGE_READWRITE );
> >     user_shared_data = addr;
> > probably should have some way to say MAP_FIXED, but I cannot find such a way. 
> > [Hint: valgrind presents such a kernel that ignores the first argument to
> > mmap() unless MAP_FIXED.]
>
> There is no reason to check for this particular allocation unless to die
> immediately if it fails. It's the special data structure [Shared User Data]
> present on all NT+ windows at the constant address. If your kernel can not
> honor this, then it's broken and have to be fixed. Or listed as incompatible
> with Wine. This area is critical to most copy-protection systems. Also lots of
> programs make a use of this data structure.

The semantics of mmap are well-defined and require applications to not assume
the segment has been mapped at the requested address unless MAP_FIXED is set.

For MEM_RESERVE allocations map_view in ntdll is called, if it's in a
"reserved" area mmap is called with MAP_FIXED, otherwise with 0. map_view
always returns an error if the returned address is not the same as the
requested one.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

--- Comment #10 from Albert Lee   2008-07-10 15:35:18 ---
Created an attachment (id=14719)
 --> (http://bugs.winehq.org/attachment.cgi?id=14719)
Add mapfile for Solaris linker to winegcc

Here is a patch that makes winegcc use a mapfile on Solaris that relaxes the
alignment restrictions for DLLs which solves the original problem. It can also
specify the address for mapping the text segment (similar to
-Wl,--section-start,.interp=0x7bf00400 etc.), but that currently crashes the
runtime loader if used.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Alexandre Julliard   2008-07-22 10:18:40 ---
Fixed by e468ab3585d3d2067350fb6596187cd5f6f5b90e.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #12 from Alexandre Julliard   2008-07-25 13:05:01 ---
Closing bugs fixed in 1.1.2.

[Bug 12783] Allocation of PEB and TEB can overlap dll mappings

http://bugs.winehq.org/show_bug.cgi?id=12783

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |e468ab3585d3d2067350fb65961
                   |                            |87cd5f6f5b90e
                 CC|                            |[e-mail]
          Component|ntdll                       |tools