Akheon23 / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Windows failure on patching LoadLibraryExW due to first instruction being jump into kernel space #128

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Build version 1.0 of tcmalloc
2. Link to any executable.
3. Run the executable

What is the expected output? What do you see instead?
Executable should run.  It sometimes does not.

What version of the product are you using? On what operating system?
Version 1.0 on Windows XP sp3

Please provide any additional information below.

So, I noticed that LoadLibraryExW is failing to be patched properly on
certain systems. Sometimes, the first instruction of the function is a
jump, and it is a jump to kernel address space which tcmalloc follows and
cannot write to and thus virtual protect fails, tcmalloc fails to patch
LoadLibraryExW, gives an error and the program exits.  Many times this
doesn't happen (when the first instruction isn't a jump into kernel space,
but I've seen it happen a few times now where the jump goes to an area that
cannot be written to. 

Has anyone else seen this?  I ran some Kaspersky antivirus scanning
software to make sure there wasn't a rootkit on my system.  

Anyway, should LoadLibraryExW occasionally contain an immediate jump into
kernel space?  Is that legitimate windows system behavior?  And what's the
best way to handle it, if it does?  

Original issue reported on code.google.com by truel...@gmail.com on 30 Apr 2009 at 12:20

GoogleCodeExporter commented 9 years ago
I have not tried 1.2 yet, but a glance at the patching code did not appear to 
address
this issue.  I hope the issue of LoadLibraryExW is due to my machine being in a 
bad
state, and not something tcmalloc has to handle. :)

Original comment by truel...@gmail.com on 30 Apr 2009 at 12:24

GoogleCodeExporter commented 9 years ago
Interesting -- I'm no windows expert, so I don't know how LoadLibraryExW is 
supposed
to behave.  Maybe someone on the google groups list (where this email is copied)
knows more?

Do the unittests that come with tcmalloc work on your machine?

Original comment by csilv...@gmail.com on 30 Apr 2009 at 5:29

GoogleCodeExporter commented 9 years ago
Now things are working again as GetProcAddress returns a spot in the loaded 
kernel32
module before the jump to kernel space, so tcmalloc can patch there.  I didn't 
get a
chance to run the unit tests when it wasn't working.  Has anyone seen this 
behavior
before?

Original comment by truel...@gmail.com on 1 May 2009 at 3:25

GoogleCodeExporter commented 9 years ago
Given this, I'm going to close it as CannotReproduce.  Feel free to open this 
again
if you see it come back.

Original comment by csilv...@gmail.com on 1 May 2009 at 3:37

GoogleCodeExporter commented 9 years ago
That is definitely not normal behavior.  The function normal entry code sets up 
for a
Win32 exception handler:

_LoadLibraryExW@12:
7C801AF5  push        34h  
7C801AF7  push        7C80E0A8h 
7C801AFC  call        __SEH_prolog (7C8024D6h)

I would say that this function has already been patched on your system.  Since 
the
jump is into kernel memory, I think that means it has been patched by a driver, 
or
some other piece of code that runs in kernel mode.  This makes me suspect that 
your
virus scanner is using this method to detect when modules are loaded in order 
to scan
them.  Of course, it's also possible that you have some malware lurking around 
and
the scan you did doesn't know about it or has already been compromised by it.

I ran Kaspersky for a while, but while it was installed any attempt to run
SysInternals Process Explorer completely locked up the machine.  Shutting it 
off did
not help, I had to completely uninstall it.  Based on this, I have the 
impression
that Kaspersky is a little more intrusive than your average AV package.

It would be unfortunate if applications using tcmalloc were simply incompatible 
with
Kaspersky.  Is the problem of the initial jump unsolvable, or just not solved?  
It
seems to me that when a function already starts with an absolute jump, there 
should
be space to replace it with another absolute jump, and later jump to the 
original
location, but I have not looked at all deeply into the mechanics of function 
patching.

Original comment by Codeben...@gmail.com on 1 May 2009 at 3:49

GoogleCodeExporter commented 9 years ago
I can reproduce this consistently now so I'd like to reopen.  Kaspersky 5 is the
problem.  Kaspersky 6 doesn't have the problem as the patched kernel32's first
instruction isn't a jump and that's what changed when it started working for 
me.  

Just run frag_unittest or many of the others to see it happen.  They return an 
error
code 1 as tcmalloc just can't patch LoadLibraryExW.

Let me know if it's possible to reopen this issue or start a new one.

Original comment by truel...@gmail.com on 7 May 2009 at 12:02

GoogleCodeExporter commented 9 years ago
Hmm, I can reopen this, but I'm not sure I can fix it.  We need to be able to 
patch
this function, and I don't think we can if the first instruction is a JMP 
(that's why
the code follows the JMPs, to get to a place where it can successfully patch).

We patch LoadLibraryExW so we can detect when new CRT libraries are loaded, and 
patch
them appropriately.  I guess we could change the code so if LoadLibraryExW 
fails, we
just give up.  This means not all libraries might run tcmalloc, even when 
tcmalloc is
installed.  That seems not ideal.

I'll ask some of the patching experts here what they think.

Original comment by csilv...@gmail.com on 7 May 2009 at 1:48

GoogleCodeExporter commented 9 years ago
Ok, we're discussing if it's possible to patch JMP instructions, and how much 
work it
would be.

It would be really helpful if you could provide a disassembly of the first 10-20
bytes of LoadLibraryExW after it's been patched by Kaspersky 5.  Is that 
possible?

Original comment by csilv...@gmail.com on 7 May 2009 at 2:47

GoogleCodeExporter commented 9 years ago
Lot's of no ops afterward which might make it easier.  :)

7C884FF1 E9 34 19 7C 2B   jmp         A804692A 
7C884FF6 90               nop              
7C884FF7 90               nop              
7C884FF8 90               nop              
7C884FF9 90               nop              
7C884FFA 90               nop              
7C884FFB 90               nop              
7C884FFC 90               nop              
7C884FFD 90               nop              
7C884FFE 90               nop              
7C884FFF 90               nop              

Original comment by truel...@gmail.com on 7 May 2009 at 6:11

GoogleCodeExporter commented 9 years ago
OK, that's a relative jump.  I was afraid of that.  It's the most annoying to 
patch,
but it's more busy-work than anything else.  I'll add it on my list of things 
to do.

Original comment by csilv...@gmail.com on 7 May 2009 at 6:33

GoogleCodeExporter commented 9 years ago
Any chance there will be time to fix for this one?  It makes tcmalloc difficult 
for
us to use on windows without it.  

Original comment by truel...@gmail.com on 1 Feb 2010 at 10:22

GoogleCodeExporter commented 9 years ago
I do not have this issue, but knowing that it exists, is not a good situation.

Last year csilvers suggested "I guess we could change the code so if 
LoadLibraryExW
fails, we just give up. [...] That seems not ideal.". 

Wouldn't this be a kind of defensive approach, if the correct fix is not doable 
in
mid-term?

Original comment by weidenri...@gmx.de on 2 Feb 2010 at 10:39

GoogleCodeExporter commented 9 years ago
It's hard to say -- my own feeling is that if we can't run LoadLibraryExW 
patching,
and some parts of the application use tcmalloc and some don't, it will be pretty
confusing and opaque to the user.

My own preferred solution here is to upgrade from Kaspersky 5 to Kaspersky 6. 
:-)  I
don't know how much work we should be doing here, to work with a single other
application, especially when that application has been upgraded so it no longer
presents a problem.

If that's not feasible, another option might be to link in your applications 
that
want tcmalloc, with a modified MSVCRT.  Then they don't require run-time 
patching,
which is inherently fragile.  This is the approach Chrome takes.  For more 
details, see
   https://groups.google.com/group/google-perftools/browse_thread/thread/41cd3710af85e57b

Original comment by csilv...@gmail.com on 4 Feb 2010 at 12:32

GoogleCodeExporter commented 9 years ago
(This is the same as I said for issue 121)

I've talked with folks who have a lot more experience with patching windows 
memory-allocation calls than I do, and they say that this kind of problem is 
endemic -- you're never going to find (or fix) them all.  The recommendation I 
got was to do libc patching instead (as described in README_windowx.txt), which 
avoids all these troubles.

So I'm going to close this WillNotFix, with a suggestion to try the libc 
patching instead if this -- or any of a myriad other potential problem-cases 
with more complex setups -- occurs.

Original comment by csilv...@gmail.com on 31 Aug 2011 at 10:09