I'm sure this will fix the panic, but I can't easily see if it's correct. For instance, if the unix domain socket code does asynchronous fd reclamation using unp_gc_task, the current thread will be some taskqueue thread. Its PID will probably be zero (but this is an implementation detail). I see some special handling for PID 0 e.g. in fuse_filehandle_get_anyflags(). Will this work the way you want in that case?

In D52625#1202173, @markj wrote:

I'm sure this will fix the panic, but I can't easily see if it's correct. For instance, if the unix domain socket code does asynchronous fd reclamation using unp_gc_task, the current thread will be some taskqueue thread. Its PID will probably be zero (but this is an implementation detail). I see some special handling for PID 0 e.g. in fuse_filehandle_get_anyflags(). Will this work the way you want in that case?

The FUSE protocol says that every request sent to the server must include the pid, gid, and uid responsible. That's so the server can do access control itself, if it wants to. The entire concept of a fuse_filehandle exists solely to support that requirement. However, it's bullshit. It's a terrible antifeature, because there are many situations in which the kernel can't possibly set those accurately. Most obviously, if a write is coming from the writeback cache. So FUSE daemons can never rely on those fields. Yet, we try to set them anyway, for compliance's sake.

Regarding this particular bug, I'm happy as long as it does not panic. This is another one of those cases where we can't possibly set the uid, gid, or pid to their "correct" values.

There's another stupid detail in the protocol too: fhid. When the kernel does FUSE_OPEN, the server may return an fhid. It's an opaque 64-bit number. But we're supposed to set it in any future request that involves the same open file. It's easily possible for two processes (or even the same process) to open a file multiple times, and we're supposed to keep track of which fhid is needed by each. That's very difficult to due given the design of our VHS. It's easy in Linux, which is probably why it was added to the fuse protocol in the first place. The main purpose, again, is server-side access control. But we have a little optimization. If two different file descriptors have the same uid, pid, gid, and access mode, then we only store one file handle for both. That's because the server almost certainly would treat them the same, for access control decisions. IIRC, that "pid == 0" expression is there precisely because of situations where we don't know the true pid. In such a case, we don't want fuse_filehandle_get_anyflags to match on pid.

So to reiterate, I'm happy as long as it doesn't panic. I'm not upset that the pid field won't be "correct".

Thanks for explanation, Alan!

Apologies for dropping this on the review after-the-fact, but I was looking over the unionfs version of the bug Alan filed (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=289700), and a couple of questions jumped out at me:

1. @glebius Is there a reason we don't just pass curthread from unp_discard()? I could imagine there might be a good reason we want to pass NULL instead of curthread, but if so that probably should be documented in VOP_OPENCLOSE.9 to reduce the likelihood of foot-shooting from FS implementations.

2. Would it be at all useful for FUSE if we were to change VOP_CLOSE (and other vnops that typically operate against file objects, eg VOP_READ/VOP_WRITE/VOP_IOCTL etc.) to also take the struct file* (if one exists, NULL otherwise) as is already done for VOP_OPEN? I realize that would be quite a lot of churn, so perhaps not worth the effort, but I also suspect something like that could be useful in cleaning up some of the especially-wrong parts of unionfs (@olce may have different thoughts here, if so I'll defer to him)

We have indeed several problems with VOP_OPEN()/VOP_CLOSE(), e.g., they are not always called in pairs, and even if they are the passed td may not match between an open and close. These interfaces need serious revision, in particular determining which exact info the filesystems would like to use and if there are better ways to obtain them.

I think longer term we should just aim to drop the td parameter to VOP_CLOSE() and probably to VOP_OPEN() as well. Actually, it's not used at all in most filesystems' VOP_CLOSE() implementation, and for VOP_OPEN() is often used to deduce some credentials, which is redundant (and perhaps sometimes inconsistent) with the struct cred that is also passed to VOP_OPEN().

From this point of view, either ensuring that td is never NULL in generic code or testing for that in the few filesystems that actually use it is fine, and which exact option is chosen does not matter.