Re: Added check for PROCESS_VM_WRITE, without which, some programs go into an infinite loop Henry Blum henry.b...@gmail.com

2010-05-09 Thread Henry Blum
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

2010-05-09 Thread Vitaliy Margolen
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

2010-05-08 Thread Charles Davis
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

2010-05-08 Thread Andrey Turkin

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?