winfsp / cgofuse

Cross-platform FUSE library for Go - Works on Windows, macOS, Linux, FreeBSD, NetBSD, OpenBSD
https://winfsp.dev
MIT License
527 stars 84 forks source link

Fix child-not-found panic in memfs lookupNode #36

Closed moroten closed 5 years ago

billziss-gh commented 5 years ago

Thanks for the PR.

Under what circumstances have you found that node == nil? Is there a way to reproduce this problem.

moroten commented 5 years ago

I was apparently too quick. When plumbing around and I did some modification that gave a path where a directory within the path did not exist. That created a panic that was recovered from in fuse/host.go. I've checked that this case is actually protected from by either the FUSE driver or some other operating system feature.

Running stat my-mount/foo/bar will first call GetAttr("/foo"), get ENOENT back and therefore skip GetAttr("/foo/bar"). I thought the operating system would try GetAttr("/foo/bar") directly and not even bother with GetAttr("/foo"). I've tried to verify this on both Windows and Linux.

I'll close this pull request because there was no bug.

billziss-gh commented 5 years ago

Thanks for the PR nevertheless.

BTW, it is possible in Windows to get a GetAttr("/foo/bar") without GetAttr("/foo") (memfs should handle this case correctly).

moroten commented 5 years ago

BTW, it is possible in Windows to get a GetAttr("/foo/bar") without GetAttr("/foo") (memfs should handle this case correctly).

Sorry for late reply, but I wanted to double check and that was well spent time. As a beginner in Go, it turned out that I had modified the wrong copy of fuse/host.go when trying to reproduce it.

Just doing dir M:\foo\bar triggers the problem on Windows. I cannot trigger it on Linux when trying stat mnt/foo/bar and running fstest.

billziss-gh commented 5 years ago

I can confirm that this problem indeed happens:

> set CGOFUSE_TRACE=*
> memfs.exe Y:
...
2019/09/08 13:04:22 [uid=197609,gid=197121]: main.(*Memfs).Getattr("/foo/bar", 0xffffffffffffffff) =
        !PANIC:runtime error: invalid memory address or nil pointer dereference

With your patch:

> set CGOFUSE_TRACE=*
> memfs.exe Y:
...
2019/09/08 13:09:18 [uid=197609,gid=197121]: main.(*Memfs).Getattr("/foo/bar", 0xffffffffffffffff) =
        (-2, &fuse.Stat_t{Dev:0x0, Ino:0x0, Mode:0x0, Nlink:0x0, Uid:0x0, Gid:0x0, Rdev:0x0, Size:0, Atim:fuse.Timespec{Sec:0, Nsec:0}, Mtim:fuse.Timespec{Sec:0, Nsec:0}, Ctim:fuse.Timespec{Sec:0, Nsec:0}, Blksize:0, Blocks:0, Birthtim:fuse.Timespec{Sec:0, Nsec:0}, Flags:0x0})

(The -2 error code is -ENOENT).

I will merge this in.

billziss-gh commented 5 years ago

BTW, I note that your name is not listed in the Contributors tab of the project: https://github.com/billziss-gh/cgofuse/graphs/contributors

I believe GitHub some times gets confused if your email address is not properly setup (either in the commit or the GitHub account). It is up to you of course whether you want this fixed or not.

If you want I can also add you to the Contributors list in the README: https://github.com/billziss-gh/cgofuse#contributors

moroten commented 5 years ago

Interestingly, the contributions on my own profile works. Doesn't really matter. I you would like, feel free to add me to the contributors list.

billziss-gh commented 5 years ago

In one other instance of this I found that GitHub did not appropriately assign contributions if the email in the commit is not the same as the one on the GitHub account. GitHub has the ability to have multiple email addresses in the account and that is how it was fixed in that instance.

I you would like, feel free to add me to the contributors list.

Done in commit df2f4be0342588a63991abf4489abc8c50eb9887.

billziss-gh commented 5 years ago

BTW, contributors are now correctly shown:

https://github.com/billziss-gh/cgofuse/graphs/contributors