Bug #999

Seekdir Bug

Added by mbalmer over 6 years ago. Updated almost 6 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

There is a bug in all seekdir()/readdir() implementations in all BSDs:

Under some circumstance, a seekdir() will no take you to the expected
position, but one further. This happens when the first entry in a
block has been unlinked and you seekdir to the second. In this case
seekdir() overshoots by one entry.

_readdir_unlocked() must not skip deleted entries when being called
from _seekdir().

A diff is attached.

- Marc Balmer

seekdir.diff Magnifier (1.56 KB) mbalmer, 05/02/2008 09:14 AM

History

#1 Updated by dillon over 6 years ago

:There is a bug in all seekdir()/readdir() implementations in all BSDs:
:
:Under some circumstance, a seekdir() will no take you to the expected
:position, but one further. This happens when the first entry in a
:block has been unlinked and you seekdir to the second. In this case
:seekdir() overshoots by one entry.
:
:_readdir_unlocked() must not skip deleted entries when being called
:from _seekdir().
:
:A diff is attached.
:
:- Marc Balmer

Thanks Marc, I will commit the patch right now.

-Matt

#2 Updated by dillon over 6 years ago

Marc and worked out the issues and I am going to commit his fix, because
it can't hurt, but unfortunately there is a DragonFly-specific issue
due to our modern readdir which translate directory entries into
a filesystem-independant form.

And, stupid me, when I created the dirent structure I didn't add enough
reserved fields to hold what we need, which is a full blown cookie for
each entry. We only have 40 bits worth of unused fields and we need
64 bits. I don't dare change that structure now, it would require
every single application ever written to be recompiled.

It may be possible to store a filename hash in the unused fields and
then match up against the hash instead of the buffer position. The
problem with that though is that it would not be perfect or even close
to perfect.

-Matt

#3 Updated by corecode over 6 years ago

Actually this is not true. The dirent structure is variable-sized
anyways, because it is only as long as necessary to store the name.

We could simply put the cookie after the name and include the cookie
length in the namelen field, so that _DIRENT_NEXT will keep working.

Of course this is only needed for binary compatibility. I think
2.0-RELEASE would be a good opportunity to have a "break" in binary
compatibility, i.e. to bump all library versions. We could then introduce
a new syscall returning nice cookies :)

cheers
simon

#4 Updated by dillon over 6 years ago

:Actually this is not true. The dirent structure is variable-sized=20
:anyways, because it is only as long as necessary to store the name.
:
:We could simply put the cookie after the name and include the cookie=20
:length in the namelen field, so that _DIRENT_NEXT will keep working.
:
:Of course this is only needed for binary compatibility. I think=20
:2.0-RELEASE would be a good opportunity to have a "break" in binary=20
:compatibility, i.e. to bump all library versions. We could then introduc=
:e=20
:a new syscall returning nice cookies :)
:
:cheers
: simon

We can't do that, programs can depend on the name length field to
figure out, well, the length of the filename.

I'm still kicking myself for not putting in 64 bits worth of reserved
fields.

-Matt
Matthew Dillon
<>

Also available in: Atom PDF