considerations over vad.End

[GoogleCodeExporter]

Rightfully so, there is some confusion / inconsistency over whether vad.End 
means the last byte in range (i.e. 0x1FFF) or the first byte out of range (i.e. 
0x2000). 

After 2.1 let's discuss how best to handle this throughout the codebase. 

Original issue reported on code.google.com by michael.hale@gmail.com on 19 Jul 2012 at 11:23

[GoogleCodeExporter]

I was under the impression it was always the last byte in the range (0x1FFF)?  
Are there instances where that's not what vad.End contains, or times where we 
use vad.End as if it were the first byte out of range?

Original comment by mike.auty@gmail.com on 22 Jul 2012 at 7:55

[GoogleCodeExporter]

Well Vad.End is a "fake" field which does not really exist - the real field is 
actually Vad.EndingVpn which does mean the very last valid PFN in the range. We 
already convert the PFN to a byte offset using the formula:

lambda x: ((x.EndingVpn + 1) << 12) - 1

The last -1 is especially in order to make the End field represent the last 
byte. But essentially nothing uses this field in this way - the most common use 
is to figure out the length of the range: vad.End - vad.Start + 1

So we should make vad.End represent the first byte _not_ in the range, and then 
the length is simply vad.End - vad.Start. This is fine because its a fake field 
- we are defining its meaning - its not coming from the underlying pdb structs.

Original comment by scude...@gmail.com on 23 Jul 2012 at 2:03

[GoogleCodeExporter]

Yeah, so just a bit of history, so we all know how this came about. Since 1.3.2 
at least, vadinfo had been doing something like 

vad_end = ((x.EndingVpn + 1) << 12) - 1

For the purpose of printing a range like 0x1000-0x1fff. But vadinfo wasn't the 
only plugin that needed the 0x1fff value, so we were duplicating the formula in 
vadwalk, vaddump (to format 0x1fff in the output file name), and various other 
places. I believe it was my decision to make the fake field and attach it to 
MMVAD.End so all 3+ plugins didn't have to duplicate the ((x.EndingVpn + 1) << 
12) - 1 formula. 

The slightly odd part is when we want to calculate the vad length and need to 
do vad.End - Vad.Start + 1 (where + 1 is only necessary because we do -1 in 
vad.End). Its just a little confusing to do -1 + 1. 

We could solve this a lot of ways, including removing the -1 from vad.End and 
then just doing -1 when we format the ending address in vadinfo. That way we 
can calculate length in other plugins with just vad.End - vad.Start. Or we can 
leave the -1 in vad.End and then create a vad.Length field (but I'm not sure we 
need *another* fake member, that's not simplifying anything). 

Its not a major bug, but with the -1 + 1 everywhere, its likely that someone 
sometime will make a mistake and we'll have an off by 1 error when dumping vad 
contents. 

Original comment by michael.hale@gmail.com on 23 Jul 2012 at 2:28

[GoogleCodeExporter]

I just want to add to this in saying that the decision to use fake fields for 
vad.Start and vad.End is the way to go since, on windows 8 this field is 
actually calculated using vad.Core.EndingVpn if the vad is of type _MMVAD and 
if it is an _MMVAD_SHORT this field is vad.EndingVpn as before. So using the 
fake field we can just make overlays for all the oses to get the exact details 
of the start and end of ranges specified.

http://code.google.com/p/volatility/source/browse/branches/scudette/volatility/p
lugins/overlays/windows/win8.py#56

A Length fake field is also a good option if it avoids confusion.

Original comment by scude...@gmail.com on 23 Jul 2012 at 4:46

[GoogleCodeExporter]

Hey guys, I'm going to commit the addition of vad.Length, a new property to 
MMVAD that represents the full length of the vad range. The point as described 
above is to prevent all the +1 and -1 in various plugins and confusion that 
might lead to off by 1 errors in plugins. The vad.End field will remain as it 
always has - the last byte in the range, not the first byte out of range. If we 
want to calculate the first byte out of range (for the purposes of dumping all 
bytes in a vad), we can easily do vad.Start + vad.Length now. I think that 
should accommodate everyone's needs? The reason I'm applying it so close to 2.1 
release is because it can simplify the patches for issue 304 and issue 306 
which do need to get committed before 2.1. Please let me know if you have 
concerns ;-)

Original comment by michael.hale@gmail.com on 24 Jul 2012 at 1:43

[GoogleCodeExporter]

This issue was closed by revision r2076.

Original comment by michael.hale@gmail.com on 24 Jul 2012 at 1:45

• Changed state: Fixed