Re: Added check for PROCESS_VM_WRITE, without which, some programs go into an infinite loop Henry Blum henry.b...@gmail.com
I looked further into the problem I set out to fix, and realize that this patch is pretty terrible. The real bug is 22006 http://bugs.winehq.org/show_bug.cgi?id=22006 and affects the game Continuum http://appdb.winehq.org/appview.php?iVersionId=3703 basically, OpenProcess doesn't enfore ACLs I am a user of the game, and the instructions that other users have provided involve a patch which allow the game to function. I was applying the patch from the git source, and wanted to try my hand at submitting a patch. The patch provided is described in the appdb page, and I thought I would improve it. That patch (not the one I submitted) wouldn't leak any handles, since it returns null before attempting NtOpenProcess when (access PROCESS_VM_WRITE). I'm not sure if that is proper behavior, but it still seems dangerous to me. Long story short, please disregard this patch, and while I will continue my efforts, any suggestions on how to approach bug 22006 would be greatly appreciated. Many thanks, Henry Blum - Original message - 08.05.2010 9:10, Henry Blum wrote: --- dlls/kernel32/process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 9a1f2f5..6dcad8f 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -2657,7 +2657,7 @@ HANDLE WINAPI OpenProcess( DWORD access, BOOL inherit, DWORD id ) if (GetVersion() 0x8000) access = PROCESS_ALL_ACCESS; status = NtOpenProcess(handle, access, attr, cid); - if (status != STATUS_SUCCESS) + if (status != STATUS_SUCCESS || (access PROCESS_VM_WRITE)) { SetLastError( RtlNtStatusToDosError(status) ); return NULL; Wouldn't this pretend operation failed for every request with PROCESS_VM_WRITE, even if NtOpenProcess succeeded, and leak a handle? Which bug should it fix?
Re: Added check for PROCESS_VM_WRITE, without which, some programs go into an infinite loop Henry Blum henry.b...@gmail.com
On 05/09/2010 01:12 PM, Henry Blum wrote: I'm not sure if that is proper behavior It's not. That patch is an outright hack specifically for your game. The proper way to fix it is to send correct ACLs to the server when new process is created. Wine still doesn't do it. Vitaliy.
Re: Added check for PROCESS_VM_WRITE, without which, some programs go into an infinite loop Henry Blum henry.b...@gmail.com
On 5/7/10 11:10 PM, Henry Blum wrote: -if (status != STATUS_SUCCESS) +if (status != STATUS_SUCCESS || (access PROCESS_VM_WRITE)) According to this, opening a process and specifying the PROCESS_VM_WRITE access right causes OpenProcess() to fail. Why do you need this? And what about programs that legitimately need to write to another process's VM? Chip
Re: Added check for PROCESS_VM_WRITE, without which, some programs go into an infinite loop Henry Blum henry.b...@gmail.com
08.05.2010 9:10, Henry Blum wrote: --- dlls/kernel32/process.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 9a1f2f5..6dcad8f 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -2657,7 +2657,7 @@ HANDLE WINAPI OpenProcess( DWORD access, BOOL inherit, DWORD id ) if (GetVersion() 0x8000) access = PROCESS_ALL_ACCESS; status = NtOpenProcess(handle, access, attr, cid); -if (status != STATUS_SUCCESS) +if (status != STATUS_SUCCESS || (access PROCESS_VM_WRITE)) { SetLastError( RtlNtStatusToDosError(status) ); return NULL; Wouldn't this pretend operation failed for every request with PROCESS_VM_WRITE, even if NtOpenProcess succeeded, and leak a handle? Which bug should it fix?