[PATCH 2/4] ntdll: Fix read_directory_getdirentries().

[Ken Thomases]

On Aug 27, 2015, at 5:05 PM, Matteo Bruni <matteo.mystral at gmail.com> wrote:
> 
> 2015-08-27 23:27 GMT+02:00 Ken Thomases <ken at codeweavers.com>:
>> On Aug 27, 2015, at 2:08 PM, Matteo Bruni <mbruni at codeweavers.com> wrote:
>>> 
>>> ---
>>> The man page for getdirentries mentions that only positions previously
>>> returned from lseek or getdirentries should be used, I'm arguably abusing
>>> the pointer to store the index and then restoring the position using
>>> SEEK_CUR. It seems to work fine for me, not sure if that might cause
>>> issues with some specific filesystem setups.
>> 
>> You're not restoring it, you're setting it to a computed value.  The computation assumes that the file position is the index of the entry that was read.  For example, you get the current position and then you're setting the position to that original position minus 2 (for the synthesized . and .. entries).  That doesn't seem like a reasonable assumption to me.
>> 
>> In some quick testing (with different code), I get results like these:
>> 
>> rc 8172 base 0/0x00000000 filepos 469762300/0x1c0000fc entries 252/0x000000fc total 252/0x000000fc
>> rc 8188 base 469762300/0x1c0000fc filepos 469762640/0x1c000250 entries 340/0x00000154 total 592/0x00000250
>> rc 2364 base 469762640/0x1c000250 filepos 469762732/0x1c0002ac entries 92/0x0000005c total 684/0x000002ac
>> 
>> The file positions seem to be 0x1c000000 with the entry index added to it.  However, each run of my test program gives a different fixed baseline. The file positions are definitely not _just_ entry indices.
>> 
>> It does seem to work to seek to an entry index in some quick testing, but it makes me very nervous.
> 
> Yes, I didn't explain it clearly but that's what I meant with the
> commit message. More below.

I did some poking through the OS X kernel code.  Different file systems seem to interpret the file position differently when reading directory entries.  So, I'm more convinced than before that assuming you can compute a file position from the desired directory entry index won't work in the general case.  The observed scheme seems to be specific to the HFS(+) driver.

> A "safer" option would be to store the position in the fd unchanged
> but reserve two values (-1 and -2?) for the '.' and '..'. I'm not sure
> that's entirely safe but I don't see anything better.

Negative values are rejected by lseek() with EINVAL.  (Didn't actually try it, but the man page says so and the kernel code seemed to agree. ;)

We would need to treat offset 0 as indicating that the next entry to be read is ".".  OFF_MAX - 1 would indicate that the next entry to be read is "..".  OFF_MAX - 2 would indicate that we should seek to 0 and read real entries.  Or something like that.

I think we should not use OFF_MAX itself as a magic value.  In my testing, the NFS driver sets the position to that at EOF.  Of course, that raises the possibility that there are other special values that we'll collide with.

-Ken