Created attachment 323977 [details] [review]
rofiles-fuse: Fix permission comparison

We want to allow write if the devinode is in the set,
not the other way around.

VERIFY_WRITE() also seems racy. You do a path-based stat check and then a path-based e.g. truncate operation, You need to open the fd in the verification, fstat on it and then do the operation based on the fd (ftruncate, fchmod, etc).

Also in general, there seem to be various edge-cases where i'm not sure rofiles-fuse does the right thing, since it always uses path based resolution even if the app did a fd-based operation. For example, a userspace fstat() call will be turned into a fstatat(basefd, path) where the path is some magical thing that fuse.c extracts from the inode nr passed in. In the case of an unlinked temp file that can easily do the wrong thing.

I'm trying to figure out if there is a way to use this to e.g. truncate (setattr SIZE) a ro file, but I can't think of any atm. Its still possibly confusing for userspace though.

Review of attachment 323977 [details] [review]:

Oh, hah.  In my defense, what I was primarily testing was open(..., O_TRUNC) which *does* work correctly because we rewrite the open flags.  This is also covered by the test suite.

Mind adding a test for this too?  (optional, feel free to commit as is)

As far as the fd vs path issues...yeah, true =/  Hm, your patch may actually make these worse as we will suddenly start enforcing things other than open(), right?

We could probably fix some of these with the FUSE lowlevel ops.  But anyways, I see rofiles-fuse as just a transient thing for functionality that should *really* live in the kernel.  See http://www.spinics.net/lists/linux-fsdevel/msg75085.html

I think you can handle the fd thing in some cases by looking at (struct fuse_file_info *)fi->fh.

Attachment 323977 [details] pushed as 89624ee - rofiles-fuse: Fix permission comparison