Re: [2/3] ntdll: Use PAGE_READWRITE access when resolving DLL imports.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
> index 5ee7d0a..314813f 100644
> --- a/dlls/ntdll/loader.c
> +++ b/dlls/ntdll/loader.c
> @@ -563,7 +563,7 @@ static WINE_MODREF *import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *d
>      protect_base = thunk_list;
>      protect_size *= sizeof(*thunk_list);
>      NtProtectVirtualMemory( NtCurrentProcess(), &protect_base,
> -                            &protect_size, PAGE_WRITECOPY, &protect_old );
> +                            &protect_size, PAGE_READWRITE, &protect_old );

Restoring the permissions is not going to work if it was already
writable.

-- 
Alexandre Julliard
julliard(a)winehq.org

[2/3] ntdll: Use PAGE_READWRITE access when resolving DLL imports.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> > NtProtectVirtualMemory( NtCurrentProcess(), &protect_base,
> > -                            &protect_size, PAGE_WRITECOPY, &protect_old );
> > +                            &protect_size, PAGE_READWRITE, &protect_old );
>
> Restoring the permissions is not going to work if it was already
> writable.

Do you mean that mprotect may fail or something else? Also, how is that
different from what current code is doing? Do you suggest to check
the old protection permissions before changing/restoring access with
NtProtectVirtualMemory()?

-- 
Dmitry.

[2/3] ntdll: Use PAGE_READWRITE access when resolving DLL imports.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > NtProtectVirtualMemory( NtCurrentProcess(), &protect_base,
> > > -                            &protect_size, PAGE_WRITECOPY, &protect_old );
> > > +                            &protect_size, PAGE_READWRITE, &protect_old );
> >
> > Restoring the permissions is not going to work if it was already
> > writable.
>
> Do you mean that mprotect may fail or something else? Also, how is that
> different from what current code is doing? Do you suggest to check
> the old protection permissions before changing/restoring access with
> NtProtectVirtualMemory()?

What I mean is that if the old protection contains WRITECOPY, you won't
be able to restore it correctly since you are going to make
NtProtectVirtualMemory reject that.

-- 
Alexandre Julliard
julliard(a)winehq.org

[2/3] ntdll: Use PAGE_READWRITE access when resolving DLL imports.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> What I mean is that if the old protection contains WRITECOPY, you won't
> be able to restore it correctly since you are going to make
> NtProtectVirtualMemory reject that.

I see. I think that a failure to restore WRITECOPY protection could be safely
ignored. My investigation shows that a section with import thunks that would
have WRITECOPY protection if it wouldn't contain import data has READWRITE
protection after the DLL is loaded.

-- 
Dmitry.

[2/3] ntdll: Use PAGE_READWRITE access when resolving DLL imports.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > What I mean is that if the old protection contains WRITECOPY, you won't
> > be able to restore it correctly since you are going to make
> > NtProtectVirtualMemory reject that.
>
> I see. I think that a failure to restore WRITECOPY protection could be safely
> ignored. My investigation shows that a section with import thunks that would
> have WRITECOPY protection if it wouldn't contain import data has READWRITE
> protection after the DLL is loaded.

OK then. A test case for this would be nice.

-- 
Alexandre Julliard
julliard(a)winehq.org