stephanelpaul / volatility

Automatically exported from code.google.com/p/volatility
GNU General Public License v2.0
0 stars 0 forks source link

kpcrscan needs some work on x64 #184

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I took a crack at fixing kpcrscan for x64. Here's a patch that solves some of 
the potential issues, but it still doesn't work. 

The patch is built on r1289 from trunk. 

Original issue reported on code.google.com by michael.hale@gmail.com on 21 Jan 2012 at 4:06

Attachments:

GoogleCodeExporter commented 9 years ago
Looks good, I think you can also use "address", which should be a native type 
of the right length, but then is just a number (so all the old checking code 
shouldn't need .v() adding behind it all).

Lastly, the additional code is to remind us that our skip function isn't very 
bright, and that we could be doing it more intelligently.  Having said that, 
it's been that way for a long, long time...

Original comment by mike.auty@gmail.com on 21 Jan 2012 at 11:23

GoogleCodeExporter commented 9 years ago
Ah thanks for the address vs Pointer tip Ikelos! I wonder why the plugin still 
doesn't work though. According to windbg, the criteria is in place:

kd> !pcr 0 
KPCR for Processor 0 at fffff80002838d00:

kd> dt _KPCR fffff80002838d00
ntdll!_KPCR
   +0x000 NtTib            : _NT_TIB
   +0x000 GdtBase          : 0xfffff800`00b95000 _KGDTENTRY64
   +0x008 TssBase          : 0xfffff800`00b96080 _KTSS64
   +0x010 UserRsp          : 0x1fcfb28
   +0x018 Self             : 0xfffff800`02838d00 _KPCR
   +0x020 CurrentPrcb      : 0xfffff800`02838e80 _KPRCB
   +0x028 LockArray        : 0xfffff800`028394f0 _KSPIN_LOCK_QUEUE
   +0x030 Used_Self        : 0x000007ff`fffde000 Void
   +0x038 IdtBase          : 0xfffff800`00b95080 _KIDTENTRY64
   +0x040 Unused           : [2] 0
   +0x050 Irql             : 0 ''
   +0x051 SecondLevelCacheAssociativity : 0x18 ''
   +0x052 ObsoleteNumber   : 0 ''
   +0x053 Fill0            : 0 ''
   +0x054 Unused0          : [3] 0
   +0x060 MajorVersion     : 1
   +0x062 MinorVersion     : 1
   +0x064 StallScaleFactor : 0x9c4
   +0x068 Unused1          : [3] (null) 
   +0x080 KernelReserved   : [15] 0
   +0x0bc SecondLevelCacheSize : 0x600000
   +0x0c0 HalReserved      : [16] 0x95067b70
   +0x100 Unused2          : 0
   +0x108 KdVersionBlock   : (null) 
   +0x110 Unused3          : (null) 
   +0x118 PcrAlign1        : [24] 0
   +0x180 Prcb             : _KPRCB

So Self still points to itself, and CurrentPrcb still points to itself + 
offset_of_Prcb 

Original comment by michael.hale@gmail.com on 21 Jan 2012 at 5:50

GoogleCodeExporter commented 9 years ago

Original comment by mike.auty@gmail.com on 12 Feb 2012 at 9:09

GoogleCodeExporter commented 9 years ago
Hey guys, 

I just made a note in Issue 190 about how it relates to this issue here. I've 
attached a patch that fixes kpcrscan on x64, but its going to need a little 
more thought before implementing, since the part that needs fixing isn't 
specific to this plugin. 

So in short, the line that fixed it is:

if (pSelfPCR & 0x0000ffffffffffff == paKCPR & 0x0000ffffffffffff and pPrcb & 
0x0000ffffffffffff == paPRCBDATA & 0x0000ffffffffffff):

I found that the self referencing member of KPCR points to 0xfffff80002838d00 
but when scanning through virtual space, volatility uses 0xf80002838d00. As 
Ikelos pointed out in issue 190, vtop(0xfffff80002838d00) == 
vtop(0xf80002838d00) but 0xfffff80002838d00 != 0xf80002838d00....and that's why 
this plugin wasn't working right. 

Original comment by michael.hale@gmail.com on 9 Apr 2012 at 3:55

Attachments:

GoogleCodeExporter commented 9 years ago
Awesome, another one bytes the dust!  5;P

I'd probably do the masking as part of the initial assignment, rather than in 
the if statement, but all the statements around there are a bit long and 
complex.  Maybe splitting the if into two lines and doing some alignment there?

So maybe:

if (blah & 0x000fffff == foo & 0x000fffff and
    bluh & 0x000fffff == faa & 0x000fffff):

I dunno, what do you think?

Original comment by mike.auty@gmail.com on 9 Apr 2012 at 4:01

GoogleCodeExporter commented 9 years ago
Yeah, that was just a proof-of-concept to show kpcrscan *does* work on x64 
without needing too much effort. I didn't intend to leave a 1-liner with 4 
0x0000ffffffffffff's ;-)

I'm wondering if there's a way to internally represent addresses/pointers on 
x64 as 0xfffff80002838d00 (do the sign extension automatically)? 

Reason to do it: If 0xfffff80002838d00 is actually what's found in KPCR.Self, 
then that's how windows uses it internally (and how windbg interprets/formats 
it also) 

Reason not to do it: like you said, vtop(0xfffff80002838d00) == 
vtop(0xf80002838d00) so not a big deal when accessing the address in a memory 
dump - and so far hasn't affected anything other than kpcrscan. 

Original comment by michael.hale@gmail.com on 9 Apr 2012 at 4:15

GoogleCodeExporter commented 9 years ago
Is it not possible to simply implement an __eq__/__neq__ method of a
pointer to mask off the first 48 bits when comparing pointers?

Original comment by scude...@gmail.com on 9 Apr 2012 at 5:01

GoogleCodeExporter commented 9 years ago
Good call. Here's a patch that implements the __eq__ check and fixes kpcrscan 
for all x64 profiles (Gleeda and I have tested). 

So there are two ways to implement the comparison in __eq__. See Ikelos's 
comment about the Intel vs AMD specs in Issue #190. So one way is to assume the 
upper 12 bits are 0's (per Intel) in which case we'd clear the upper 12 bits of 
both pointers before comparison. The other way is to assume the upper 12 bits 
should be sign extensions of bit 47 (per AMD) in which case we do the sign 
extension before comparison. My patch uses the AMD method, but if you guys 
prefer it a different way, feel free to modify it. 

I also left a debug statement in the patch so that I could see when running 
other plugins if and when the Pointer.__eq__() method was being called. It 
turns out the only location I could find was inside the sockets command. In 
this case I got an error (see below) which is why I added the check if offset 
is None at the top of Pointer.__eq__(). 

{{{
$ python vol.py -f ~/Desktop/memory/silentbanker.vmem sockets 
Volatile Systems Volatility Framework 2.1_alpha
 Offset(V)  PID    Port   Proto               Address        Create Time               
---------- ------ ------ ------------------- -------------- 
-------------------------- 
Traceback (most recent call last):
  File "vol.py", line 173, in <module>
    main()
  File "vol.py", line 164, in main
    command.execute()
  File "/Users/Michael/volatility_kpcrscan/volatility/commands.py", line 101, in execute
    func(outfd, data)
  File "/Users/Michael/volatility_kpcrscan/volatility/plugins/sockets.py", line 41, in render_text
    for sock in data:
  File "/Users/Michael/volatility_kpcrscan/volatility/win32/network.py", line 183, in determine_sockets
    for entry in table:
  File "/Users/Michael/volatility_kpcrscan/volatility/obj.py", line 692, in __iter__
    if (self.current == None):
  File "/Users/Michael/volatility_kpcrscan/volatility/obj.py", line 623, in __eq__
    if other & 1 << 47:
TypeError: unsupported operand type(s) for &: 'NoneType' and 'int'
}}}

I'm not sure if there's a better way of preventing the TypeError than my check 
at the top of Pointer.__eq__(). Ikelos, can you give it a look? Then either 
commit if you're comfortable with it or let me know and I'll do it!

Original comment by michael.hale@gmail.com on 8 May 2012 at 8:29

Attachments:

GoogleCodeExporter commented 9 years ago
Hmmm, so looking at the patch, there's a couple of things I'd consider.  So, 
you're checking if bit 47 is set, and then setting all the others, but if it's 
not set, then bits 64 through 48 could be anything (therefore two equivalent 
pointers that both have 0 at bit 47 could fail to be equivalent).  That leads 
me to question whether the high bits are necessary in the comparison.  If we're 
intel they'll be 0's for both, and if we're amd64, they'll be extensions of bit 
47 (so therefore bit 47 is enough to determine equality).  So in the equality 
test we should always mask off everything above bit 47 because it simply 
doesn't matter to the value.

The question remaining is whether we should instead always return a masked off 
value for .v(), and therefore get the equality function for free.  That 
probably won't work in instances where people end up comparing against a number 
(that does have additional bits), rather than against a pointer (which would 
always get masked).

So do please commit it, but make the equality function something along the 
lines of:

def __eq__(self, other):
  if other == None:
    return False
  return (0xffffffffffff & self.v()) == (0xffffffffffff & other)

Then it looks good to go...  5;)

Original comment by mike.auty@gmail.com on 8 May 2012 at 10:47

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1681.

Original comment by michael.hale@gmail.com on 8 May 2012 at 11:13

GoogleCodeExporter commented 9 years ago
Just FYI:
http://scudette.blogspot.ch/2012/10/finding-kpcr-in-memory-images.html

Original comment by scude...@gmail.com on 11 Oct 2012 at 2:02