zfs: Correct reading of .zfs directory

[manu]

Summary

With the current implementation reading the .zfs directory with getdents
or getdirentries will behave weirdly :

• First call will fill the '.' dirent info and return the number of bytes transfered
• Second call will fill the '..' dirent info and return the number of bytes transfered
• Third call will fill the 'snapshot' dirent info and return the number of bytes transfered
• Fourth call will fill again the 'snapshot' dirent info, return -1 and set errno to EINVAL

As noted in the getdents man when the end of the directory is reached a value
of 0 should be returned.

Correct the implementation by filling the buffer the 'snapshot' info only when
the fd offset is equal to the dots offset (the size of two dirent structures).

Sponsored by: Gandi.net
MFC After: 3 days

Test Plan

Test with this little crappy program ;
#include <stdio.h>
#include <fcntl.h>
#include <dirent.h>
#include <errno.h>

int main(int argc, char **argv)
{
int fd, ret, i;
struct dirent dir;

fd = open(argv[1], O_RDONLY);

while ((ret = getdents(fd, (char *)&dir, sizeof(dir))) != 0) {

		printf("%s\n", dir.d_name);
		if (ret == -1) {
			printf("errno: %d\n", errno);
			break;
		}

}
}

$ cc blah.c
$ ./a.out /.zfs
.
..
snapshot
snapshot
errno: 22

After the patch :
$ ./a.out /.zfs
.
..
snapshot

Also truss ls /.zfs.

[avg]

Thank you for catching this!

This looks good to me but I wander if it would be still possible to detect invalid offsets rather than treating them as EOD.
Could you please consider if it's possible and easy to precalculate the offset just after the "snapshot" entry, so that we could see if we got the valid end-of-directory offset or some bogus offset?

[manu]

Ensure we aren't reading in a bogus offset.

[avg]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c

689

I think that this is the same as dots_offset, so this check is redundant.

710

I think that the correct predicate for this check is !=.

[manu]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c

689

Redudant with which check ?
I agree that the correct one should be < dots_offset though.

710

Yes agreed.

[avg]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c

689

With the two following checks:

• uio->uio_offset == dots_offset
• uio->uio_offset != (3 * sizeof(entry))

[manu]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c

689

Ah yes with changing the last check to != it make sense no, I'll update the review.

[manu]

No plan to update this now.