Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Size of syscall.Dirent not always consistent with FUSE #197

Closed
lechner opened this issue Jan 25, 2018 · 7 comments
Closed

Size of syscall.Dirent not always consistent with FUSE #197

lechner opened this issue Jan 25, 2018 · 7 comments

Comments

@lechner
Copy link
Contributor

lechner commented Jan 25, 2018

Hi,

This is from Debian packaging. On i386 the Getdents test in version 1.4.2 fails because the syscall.Dirent data structure gocryptfs uses has inconsistent sizes between architectures.

While the inode and offset members can vary in scenarios like this, they are not the issue here. The golang runtime uses only the getdents64 syscall and via cgo tweaks struct dirent to its liking (although I might have preferred struct dirent64 ). The issue here are the data types for name length and file type, as well as the padding for alignment.

On all architectures, both go-fuse and libfuse return at most 280 bytes. On amd64, that size matches the syscall.Dirent data type gocryptfs uses, but on i386 the latter drops to 276 bytes. It causes an error in internal/syscallcompat/getdents_linux.go:

if int(s.Reclen) > sizeofDirent {
    tlog.Warn.Printf("Getdents: corrupt entry #%d: Reclen=%d > %d. Returning EBADR",
        numEntries, s.Reclen, sizeofDirent)
    return nil, syscall.EBADR
}

Here is a small program to illustrate the various sizes. This is the output for amd64:

gocryptfs
unsafe.Sizeof (syscall.Dirent{})                 =  280

go-fuse
unsafe.Sizeof (gofuse_Dirent{}) + 256            =  280

libfuse
sizeof (struct libfuse_dirent) + 256             =  280

readdir(3), provided by libc
sizeof (struct dirent)                           =  280
sizeof (struct dirent64)                         =  280

getdents(3) syscall, not wrapped by libc
sizeof (struct linux_dirent) with 256[]          =  280

proposal to bring go-fuse in line with gocryptfs
unsafe.Sizeof (PROPOSED_gofuse_Dirent{}) + 256   =  280

For i386, the sizes are not nearly as consistent:

gocryptfs
unsafe.Sizeof (syscall.Dirent{})                 =  276

go-fuse
unsafe.Sizeof (gofuse_Dirent{}) + 256            =  280

libfuse
sizeof (struct libfuse_dirent) + 256             =  280

readdir(3), provided by libc
sizeof (struct dirent)                           =  276
sizeof (struct dirent64)                         =  276

getdents(3) syscall, not wrapped by libc
sizeof (struct linux_dirent) with 256[]          =  268

proposal to bring go-fuse in line with gocryptfs
unsafe.Sizeof (PROPOSED_gofuse_Dirent{}) + 256   =  276

Which data structure should be used in gocryptfs? Bonus question: Is FUSE safe on i386? Thank you!

@lechner lechner changed the title syscall.Dirent structure has inconsistent sizes between architectures Size of syscall.Dirent is inconsistent between architectures Jan 25, 2018
@lechner lechner changed the title Size of syscall.Dirent is inconsistent between architectures Size of syscall.Dirent is inconsistent with corresponding data in FUSE Jan 25, 2018
@lechner lechner changed the title Size of syscall.Dirent is inconsistent with corresponding data in FUSE Size of syscall.Dirent is inconsistent with FUSE Jan 25, 2018
@lechner lechner changed the title Size of syscall.Dirent is inconsistent with FUSE Size of syscall.Dirent not always consistent with FUSE Jan 25, 2018
@rfjakob
Copy link
Owner

rfjakob commented Jan 25, 2018

Wow, good find.

The FUSE side of things should be fine, as it has its own fuse_dirent definition at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/fuse.h?h=v4.12#n707 .

struct fuse_dirent {
	uint64_t	ino;
	uint64_t	off;
	uint32_t	namelen;
	uint32_t	type;
	char name[];
};

This is independent of what the getdents64 syscall uses.

Looks like the syscall.Dirent definition of i386 is wrong. Will check.

@rfjakob
Copy link
Owner

rfjakob commented Jan 25, 2018

syscall.Dirent (definition) seems to match what Linux has (definition) - or at least it is NOT smaller than what Linux has.

What is Reclen in the warning you get?

@lechner
Copy link
Contributor Author

lechner commented Jan 25, 2018

Like you I think golang does the right thing.

The print statement unintentionally reverses the values (my quote was patched) but it was that s.Reclen from FUSE is 280 while the space reserved via syscall.Dirent was 276.

Here is the build log.

@lechner
Copy link
Contributor Author

lechner commented Jan 25, 2018

I am thinking about using Linux's FUSE dirent until go-fuse exports its _Dirent type. Would you like to see a pull request? Thank you!

@rfjakob
Copy link
Owner

rfjakob commented Jan 25, 2018 via email

rfjakob added a commit that referenced this issue Jan 25, 2018
We used to print somewhat strange messages:

	Getdents: corrupt entry #1: Reclen=276 > 280. Returning EBADR

Reported at #197
@rfjakob
Copy link
Owner

rfjakob commented Jan 25, 2018

It looks like the assumption that reclen cannot be longer than the Dirent is just wrong. Linux can insert arbitrary padding in between. And it does, to improve alignment, apparently: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/readdir.c?h=v4.15-rc9#n173

rfjakob added a commit that referenced this issue Jan 25, 2018
Due to padding between entries, it is 280 even on 32-bit architectures.
See #197 for details.
@rfjakob
Copy link
Owner

rfjakob commented Jan 25, 2018

Fixed by f3838c0 .

@rfjakob rfjakob closed this as completed Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants