[PATCH] ntdll: Add a shortcut to NtReadVirtualMemory for the local process

[Martin Storsjö]

On Tue, 29 Oct 2019, Andrew Wesie wrote:

> On Tue, Oct 29, 2019 at 9:04 AM Martin Storsjo <martin at martin.st> wrote:
>> ...
>> Therefore, exception handling in executables with libunwind on i686
>> doesn't work when run with wine within docker.
>
> I personally don't think this is a great rationale for changing the
> behavior of wine. It sounds like a rationale for docker to allow
> SYS_PTRACE.

It can be enabled, but it's not generally enabled by default.

In any case: I believe it is fair to hope that 
EnumProcessModules(GetCurrentProcess()) would work, without privileges 
that let you read the memory of other processes.

Exactly how to achieve that is deabatable though. The same end result can 
be achieved by adding a special case for the local process within 
EnumProcessModules itself as well. That leads to a bit more code 
duplication, and only gives this benefit to exactly this one function and 
nothing else though.

> But I am not a wine maintainer, so my only real concern is
> that the correctness of NtReadVirtualMemory is not affected as it is
> used by anti-cheat and other programs.

That's a fair concern. I wasn't very familiar with this function from the 
beginning, and this was my initial attempt at achieving my goal with a 
minimum of code duplication and giving the same benefit to other 
introspection functions as well (in case there are any).

>
>> ...
>> +            server_enter_uninterrupted_section( &csVirtual, &sigset );
>> +            if (virtual_check_buffer_for_read( addr, size ))
>> +            {
>> +                memcpy(buffer, addr, size);
>> +                status = STATUS_SUCCESS;
>> +            }
>
> This does not have the same semantics as the original code. The
> original code used kernel system calls (e.g. pread) which will not
> cause a page fault. virtual_check_buffer_for_read explicitly reads
> each page to trigger a fault. At a minimum, this causes the behavior
> to diverge from Windows for guard pages. It may have other differences
> as well.
>
> A potential fix may be to implement something similar to
> virtual_uninterrupted_read_memory.

FWIW, I tried using virtual_uninterrupted_read_memory for doing the 
reading in NtReadVirtualMemory if the target process was the current one, 
but that turned out to fail on some of the memory regions that 
EnumProcessModules reads, as some of the views it reads have VPROT_SYSTEM 
set, and virtual_uninterrupted_read_memory doesn't read those. I'm not 
familiar with these internals to have much clue about what to make of that 
though.

// Martin