macos-fuse-t / fuse-t

Other
929 stars 7 forks source link

`lookup` reply handling different than libfuse/osxfuse #72

Closed vvolkl closed 3 weeks ago

vvolkl commented 3 weeks ago

In our filesystem, we see that a ls /mntpoint/subdir/NONEXISTINGDIRECTORY shows a listing of the filesystem root (when it should show "No such file or directory"). My hunch is that fuse-t doesn't honor negative replies of lookup with ino=0 ( see https://github.com/libfuse/libfuse/blob/master/include/fuse_lowlevel.h#L63 ) and thinks it's a positive reply.

I can provide further details and reproducers if needed. This is the last thing blocking us from using fuse-t, any help is much appreciated!

macos-fuse-t commented 3 weeks ago

I will look into it

macos-fuse-t commented 3 weeks ago

I tried to reproduce the issue with the loopback fs sample (libfuse/example/fusexmp_fh) and it works as expected. Please provide me more details or attach a sample code that explains it.

vvolkl commented 3 weeks ago

Thanks for taking a look! That may not be the right example to look at, as it doesn't use the low-level API which lookup is part of. There's only the hello_ll example which is apparently too simple to include this particular usecase, but I think I can adapt it.

Here is the actual production code - we do a normal reply with inode 0 instead of a fuse_reply_err because of the potential negative caching mentioned in the libfuse source

I think it'd be fine to just treat the fuse_reply_entry with ino=0 the same way as a fuse_reply_err to be consistent with macfuse and libfuse.

But let me write a quick reproducer.

vvolkl commented 3 weeks ago

Full reproducer: https://gist.github.com/vvolkl/b98679607db1793845703d80d10f877f

So here is a reproducer - slightly weird example, but it should illustrate my point about fuse-t not being consistent with other implementations. This is based on hello_ll.c from fuse-2.9.9. The main change is that lookup now returns a negative reply in the form of ino=0 instead of a fuse_reply_err ( for any file that is not "hello", as this is still the hello world example )

static void hello_ll_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
{
    struct fuse_entry_param e;

    if (parent != 1 || strcmp(name, hello_name) != 0) {
        memset(&e, 0, sizeof(e));
        e.ino = 0;
        fuse_reply_entry(req, &e);
    } else {
        memset(&e, 0, sizeof(e));
        e.ino = 2;
        e.attr_timeout = 1.0;
        e.entry_timeout = 1.0;
        hello_stat(e.ino, &e.attr);

        fuse_reply_entry(req, &e);
    }
}

The rest of the changes are just removing some checks so we get a better contrast for fuse-t.

Both libfuse on linux and osxfuse treat the reply similar to an error:

~/r/libfuse/example ((fuse-2.9.9))$ cd
~$ gcc -Wall lookup_negative.c `pkg-config fuse --cflags --libs` -o lookup_negative
~$ ./lookup_negative /tmp/hello
#in another shell
~$ ls /tmp/hello
hello
~$ ls /tmp/hello/nosuchdir
ls: cannot access '/tmp/hello/nosuchdir': No such file or directory

Similar with osxfuse:

cernvm-mac-mini-1:~ sftnight$ clang -D_FILE_OFFSET_BITS=64 -lfuse -I /usr/local/include/fuse lookup_negative.c  -o lookup_negative
cernvm-mac-mini-1:~ sftnight$ ./lookup_negative /tmp/hello
#in another shell
cernvm-mac-mini-1:~ sftnight$ ls /tmp/hello
hello
cernvm-mac-mini-1:~ sftnight$ ls /tmp/hello/nosuchdir
ls: /tmp/hello/nosuchdir: No such file or directory

However with fuse-t:

sftnight@cernvm-mac-m1-3 example % clang -I/Library/Application\ Support/fuse-t/include/fuse -L/usr/local/lib -lfuse-t -D_FILE_OFFSET_BITS=64 lookup_negative.c -o lookup_negative
sftnight@cernvm-mac-m1-3 example % ./lookup_negative /tmp/hello 
sftnight@cernvm-mac-m1-3 ~ % ls /tmp/hello
hello
sftnight@cernvm-mac-m1-3 ~ % ls /tmp/hello/nosuchdir
sftnight@cernvm-mac-m1-3 ~ % 

(A more minimal reproducer is just updating lookup as above, and adding a case in hello_stat:

        case 0:
                stbuf->st_mode = S_IFREG | 0444;
                stbuf->st_nlink = 1;
                stbuf->st_size = strlen(hello_str);
                break;

This will let fuse-t say:

sftnight@cernvm-mac-m1-3 ~ % ls /tmp/hello/nosuchdirdd
ls: fts_read: Permission denied

)

So short summary: for lookup, macfuse and libfuse treat a fuse_reply_entry with ino=0 like an error reply, fuse-t doesn't and it would be good to be consistent.

macos-fuse-t commented 3 weeks ago

I cannot reproduce it. This is what I did:

  1. Changed hello_ll.c sample in the libfuse/example folder so the lookup looks like this:

     static void hello_ll_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
     {
        struct fuse_entry_param e;
    
        if (parent != 1 || strcmp(name, hello_name) != 0) {
               memset(&e, 0, sizeof(e));
                e.ino = 0;
                fuse_reply_entry(req, &e);
        }
        else {
                memset(&e, 0, sizeof(e));
                e.ino = 2;
                e.attr_timeout = 1.0;
                e.entry_timeout = 1.0;
                hello_stat(e.ino, &e.attr);
    
                fuse_reply_entry(req, &e);
        }
    }
  2. Compiled with cmake
  3. Mounted with ./example/hello_ll -f /tmp/test
  4. Executed:
    /tmp % ls /tmp/test
    hello
    /tmp % ls /tmp/test/nosuchfile
    ls: /tmp/test/nosuchfile: No such file or directory

    Am I missing something?

vvolkl commented 3 weeks ago

Like this you wouldn't see a difference, because the hello_stat function is written in a way that it returns an error that looks the same. You can try changing it to remove the inode check as I suggested above, or, perhaps even simpler, add the following debug printout in hello_stat:

printf("hello_stat, ino: %lu\n", ino);

If you run the mount in the foreground, you should see that with fuse-t hello-stat is called with inode 0, but both libfuse and osxfuse have already failed the lookup and will never get to hello_stat for a non-existing dir with inode 0. Sample output for me (fuse-t, while doing a ls /tmp/hello/nonexistingdir):

sftnight@cernvm-mac-m1-3 example % ./lookup_negative3 /tmp/hello                                                                                                                  
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 0
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 1
hello_stat, ino: 0
hello_stat, ino: 0
hello_stat, ino: 0
hello_stat, ino: 0

Sample output for me (libfuse/linux, while doing a ls /tmp/hello/nonexistingdir):

~/r/libfuse/example ((fuse-2.9.9))$ ./lookup_negative3 /tmp/hello
hello_stat, ino: 1
hello_stat, ino: 2
hello_stat, ino: 2
hello_stat, ino: 2
macos-fuse-t commented 3 weeks ago

fixed

vvolkl commented 3 weeks ago

Great, thank you!