fcntl(2): add F_KINFO operation

[kib]

Summary

that returns struct kinfo_file for the given file descriptor.  Among
other data, it also returns kf_path, if file op was able to restore file
path.

[markj]

Why not a sysctl-based interface, where the fd is a component of the MIB? Then info for a specific fd can be grabbed from a different process.

sys/kern/kern_descrip.c

860

I suspect it should be at least cap_fcntl_rights. Maybe this should not be permitted in capability mode at all, it seems dubious to allow a capmode process to look up the path corresponding to an fd.

[kib]

In D33277#752190, @markj wrote:

Why not a sysctl-based interface, where the fd is a component of the MIB? Then info for a specific fd can be grabbed from a different process.

There is already kern.proc.filedesc, and I highly doubt that somebody needs a path for individual fd of remote process.

[kib]

Disable F_KINFO in capability mode.
Use cap_fcntl_rights.

[kib]

sys/kern/kern_descrip.c

860

So I added this, but an after-thought is that I do not see anything wrong with resolving full path even in capability mode. What additional privilege does this resolution grant?

861

Should this be ECAPMODE or ENOTCAPABLE?

[markj]

In D33277#752192, @kib wrote:

In D33277#752190, @markj wrote:

Why not a sysctl-based interface, where the fd is a component of the MIB? Then info for a specific fd can be grabbed from a different process.

There is already kern.proc.filedesc, and I highly doubt that somebody needs a path for individual fd of remote process.

I'm not so sure, it could be useful for some kind of truss extension which prints a path corresponding to an fd.

The sysctl-based interface is also slightly more future-proof since it lets the caller and kernel detect size mismatches.

lib/libc/sys/fcntl.2

194 ↗

(On Diff #99493)

sys/kern/kern_descrip.c

860

It doesn't grant any new privileges, but it leaks information about a namespace. KERN_PROC_FILEDESC is not permitted in capability mode, for example, and the fd may have been opened by a process with more privileges than the current (sandboxed) process. As another example, we have CAP_GETPEERNAME for a socket, but this operation returns a superset of the information provided by getpeername(2), so the right can be bypassed.

Hmm, I forgot that we have cap_fcntls_limit(2) and fget_fcntl(). So with a bit more work it could be possible to strip the right to call F_KINFO for a particular fd. If that is implemented, then I think it is ok to remove the IN_CAPABILITY_MODE() check. The code should also check for CAP_GETPEERNAME for a socket, though.

861

ECAPMODE means, "this operation is not permitted in capability mode," whereas ENOTCAPABLE means "this fd has insufficient rights to perform the operation," so ECAPMODE is correct. If we added a fine-grained right(4) for this operation, like CAP_FCNTL_KINFO, and it was missing, then the error would be ENOTCAPABLE.

865

This should be zeroed, the fo_fill_kinfo() implementations do not handle it and we'll copy out uninitialized pad bytes.

[kib]

In D33277#752216, @markj wrote:

I'm not so sure, it could be useful for some kind of truss extension which prints a path corresponding to an fd.

Our truss or a debugger should be fine using kern.proc.filedesc, it is not performance-important.

The sysctl-based interface is also slightly more future-proof since it lets the caller and kernel detect size mismatches.

It is not, unfortunately. What can be done, on the other hand, is to require kf_structsize to be pre-filled with the expected size before fcntl call.

[kib]

Require userspace to fill kf_structsize for versioning.

[damjan.jov_gmail.com]

In D33277#752192, @kib wrote:

In D33277#752190, @markj wrote:

Why not a sysctl-based interface, where the fd is a component of the MIB? Then info for a specific fd can be grabbed from a different process.

There is already kern.proc.filedesc, and I highly doubt that somebody needs a path for individual fd of remote process.

In Wine's case, other processes would be ok, because they share all file descriptors for "Windows files" with wineserver (with sendmsg() + SCM_RIGHTS), and this F_KINFO would only get queried in wineserver.

Even if sysutils/lsof takes the route of doing more from user space, kern.proc.filedesc should be enough for its needs. In fact its upstream developers were already in discussion about just that. [Its current approach of probing kernel data structures with kvm_read() is racy and unmaintainable, "struct namecache" changes ABI once a year, zfs broke in FreeBSD >= 13, msdosfs returns corrupt filenames, isofs was broken from FreeBSD 6.]

I like this patch.

[markj]

sys/kern/kern_descrip.c

875

Should this be using export_file_to_kinfo() instead of calling the file method directly? It fills out some generic fields.

[kib]

Use export_file_for_kinfo.
Lock fdp around export_file.
Use fget_cap_locked().

[kib]

Slightly streamline flow.

[jhb]

@brooks so he is aware of of this when it comes down to merging to CheriBSD. (Maybe sys/fcntl.h should be a default pause path in CheriBSD's mergify config?)

lib/libc/sys/fcntl.2

56 ↗

(On Diff #99583)

Maybe just say the type of the 3rd arg depends on the operation (and document the type for each operation that takes the third arg) instead of trying to assume it's a single type? In CheriBSD we have a wrapper in libc to deal with this and have fcntl() as a truly var-args function. On existing platforms var-args is close enough to work for passing syscall arguments, but not all ABIs require this. (var-args are passed as a block in a bounded pointer on CHERI platforms and not just a continuation of register arguments).

[kib]

lib/libc/sys/fcntl.2

56 ↗

(On Diff #99583)

I pushed everything but this change, the review is updated with the man page changes.

[kib]

fcntl(2): be more precise about third arg type

[emaste]

lib/libc/sys/fcntl.2

57 ↗

(On Diff #99607)

maybe "with type dependent on the specific operation"

or maybe

can take an additional third argument
.Fa arg.
Unless otherwise noted below for a specific operation,
.Fa arg
has type
.Vt int .

295 ↗

(On Diff #99607)

Is "command" or "operation" the better term, in general? I think we should be consistent between this sentence and the added text above/below

307 ↗

(On Diff #99607)

These commands take a pointer to struct flock as the third argument arg.
or
These advisory file locking commands take a pointer to struct flock as the third argument arg.

[kib]

Add Ed suggestions.

[emaste]

lib/libc/sys/fcntl.2

305 ↗

(On Diff #99610)

Maybe "operations" here too for consistency?
But from the context this one is clear so I don't mind either way.

[kib]

One more "operations".