Add AT_EMPTY_PATH

[kib]

Summary

It is currently allowed to fchownat(2), fchmodat(2), fchflagsat(2), utimensat(2), fstatat(2), and linkat(2).

For linkat(2), PRIV_VFS_FHOPEN privilege is required to exercise the flag. It allows to link any open file.

[kib]

Allow AT_EMPTY_PATH for fstatat(2).
Fix visibility issue.

[kib]

fstatat(2) needs locked leaf vnode.

[kib]

Skip ENOTDIR for EMPTYPATH

[trasz]

Seems to work perfectly for at least fstatat(2) and fchownat(2). Thanks for working on it!

[kib]

Update man page for fstatat(2). Other will follow after this update is corrected.

[emaste]

lib/libc/sys/stat.2

114–120

My take:

If the path argument is an empty string, operate on the file or directory referenced by the descriptor fd. If fd is equal to AT_FDCWD, operate on the current working directory.

"operate on" could be "return information about" if that's more clear.

To me it seems that "file or directory referenced..." is sufficiently clear that the "does not need to be a directory" sentence could be omitted, but I'm not certain - you could also leave it.

[kib]

Reword manual page update, by Ed.

[emaste]

Man page change looks fine to me, would be good to have a 2nd opinion from @trasz or manpages

[markj]

What is the use-case for this? fstatat(fd, "", &sb, AT_EMPTY) seems equivalent to fstat(fd, &sb).

lib/libc/sys/stat.2

124

Some mention of the required privileges seems useful.

sys/kern/vfs_syscalls.c

1585

Why is the check done after resolution?

[kib]

In D29111#655384, @markj wrote:

What is the use-case for this? fstatat(fd, "", &sb, AT_EMPTY) seems equivalent to fstat(fd, &sb).

For fstatat(2) yes, but note linkat(2). The real user is linuxolator, anyway. Trasz@ needs this to make some linux program work.

lib/libc/sys/stat.2

124

There is no required privilege for fstat(). For linkat() I will mention PRIV_VFS_FHOPEN

sys/kern/vfs_syscalls.c

1585

Because I do not know if namei() would really use EMPTYPATH in advance, and I do not want to duplicate the logic.

[markj]

In D29111#655388, @kib wrote:

In D29111#655384, @markj wrote:

What is the use-case for this? fstatat(fd, "", &sb, AT_EMPTY) seems equivalent to fstat(fd, &sb).

For fstatat(2) yes, but note linkat(2). The real user is linuxolator, anyway. Trasz@ needs this to make some linux program work.

I see, there is some context in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=197778

[trasz]

Also, given how many Linux apps started to use those (visible as a number of regressions in Ubuntu Focal compared to Ubuntu Bionic), it might soon come really handy when porting software.

[emaste]

Also, given how many Linux apps started to use those

Maybe happening via glibc though, not explicit AT_EMPTY_PATH in src?

[markj]

sys/kern/vfs_lookup.c

472

I think a comment explaining how this is handled by the caller would be useful. Else it looks like a bug.

I would go further and centralize error handling at the end of the function:

cnp->cn_nameptr = cnp->cn_pnbuf;
return (0);

fail:
if (error != ENOENT)
    namei_cleanup_cnp(cnp);
return (error);

Otherwise this is IMO too fragile.

587

IMO it would be nicer to lift this into a separate function. There is too much indentation otherwise.

[kib]

sys/kern/vfs_lookup.c

472

I restructured the error handling to do cleanup in the caller instead. Also added namei_emptypath() and reduced indentations.

[kib]

namei_emptypath()

[markj]

Should faccessat(2) handle the flag as well?

sys/kern/vfs_lookup.c

469

The comment should be updated to note that EMPTYPATH is handled in the caller. Otherwise it looks like the code is wrong.

622

Cleanup now needs to happen here too, no?

[kib]

sys/kern/vfs_lookup.c

622

I think that no. To get there, we must already pass namei_getpath() above, and then ENOENT or any other error is already handled.

Hmm, indeed, userspace might change the string between previous and this namei_getpath(). Ok.

[kib]

Enable for faccessat(2).
Handle one more namei_getpath() call.
Update comment.

[markj]

Seems ok to me. I did some fuzzing of the change without any problems found so far.

[kib]

Update man pages for the affected syscalls.

[markj]

lib/libc/sys/link.2

120 ↗

(On Diff #86561)

I guess this should be fd1.

129 ↗

(On Diff #86561)

name2?

131 ↗

(On Diff #86561)

I believe it should be fd2.

[kib]

link(2) arg names fixes

[markj]

sys/kern/vfs_lookup.c

322

I believe it should not be allowed: fooat(AT_FDCWD, "", AT_EMPTY_PATH) is an operation on the CWD, but the CWD is not a capability. fooat(AT_FDCWD) is always disallowed in capability mode.

[kib]

Remove XXX comment about AT_FDCWD for AT_EMPTY_PATH

[markj]

sys/kern/vfs_syscalls.c

1585

Actually I am confused by this. Shouldn't we be handling NIRES_EMPTY path for the first path, not the second?

[kib]

Check privilege for the right lookup case in linkat(AT_EMPTY_PATH).

[wulf]

In D29111#654477, @trasz wrote:

Seems to work perfectly for at least fstatat(2) and fchownat(2). Thanks for working on it!

faccessat(2) seems to work with O_PATH file descriptors too

[markj]

lib/libc/sys/link.2

131 ↗

(On Diff #86561)

Eh, sorry it should be name1 and fd1. I misunderstood the interface the first time I read this.

[kib]

I rebased O_PATH branch onto AT_EMPTY_PATH, so for now I will no longer update these reviews.
I plan to commit after Peter' tests.

[markj]

sys/kern/vfs_syscalls.c

1563

It looks like the committed version is still doing the priv_check() on the second path.