Page MenuHomeFreeBSD

O_PATH: allow vfs_extattr syscalls
ClosedPublic

Authored by greg_unrelenting.technology on Sun, Oct 10, 1:00 PM.

Details

Summary

These calls do operate on vnodes only, not file contents.
This is useful for e.g. the xdg-document-portal fuse filesystem.


This should be fine, right?


related: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198570#c3

Test Plan

I've only tested that a tiny program that does extattr_set_fd(open("yeet", O_PATH), EXTATTR_NAMESPACE_USER, "memes", "lol", sizeof("lol")); no longer fails and the result is applied (getextattr user memes yeet)

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I think this is fine, mostly because each extrattr_XXX_fd variant you change also has the extattr_XXX_file version which takes the path.

But extattr(2) man page should be updated noting the allowance of O_PATH file descriptors.

This revision is now accepted and ready to land.Sun, Oct 10, 6:23 PM
This revision now requires review to proceed.Mon, Oct 11, 10:07 AM
This revision is now accepted and ready to land.Mon, Oct 11, 12:05 PM

Please provide the full 'Author:' line for the commit. Or better, mail me the git-format-patch for the change.

What does O_PATH help with here? I'm having trouble understanding from the linked PR. extattr operations have to pass the extattr_check_cred() test, and if a caller passes that test then why would it have bothered using O_PATH in the first place?

These calls do operate on vnodes only, not file contents.

So does fchmod(2).

What does O_PATH help with here? I'm having trouble understanding from the linked PR. extattr operations have to pass the extattr_check_cred() test, and if a caller passes that test then why would it have bothered using O_PATH in the first place?

First, look at it from the different angle. Why disable operation that does not have undesired effects?

Second, using O_PATH fd instead of normal fd does provide a delicate change of semantic. For instance, if you want take a bookmark for a file, for which you want to add or remove some extended attribute later, opening it for write now would prevent its execution. Opening it with O_PATH, and you can both execute and modify attributes, without loosing the file to a race with parallel rename or even ABA kind of substitution.

These calls do operate on vnodes only, not file contents.

So does fchmod(2).

In D32438#731895, @kib wrote:

Please provide the full 'Author:' line for the commit. Or better, mail me the git-format-patch for the change.

hm, I thought phab can handle this, at least sometimes I've managed to get the Author line with arc patch

Anyway, https://dl.unrelenting.technology/0001-O_PATH-allow-vfs_extattr-syscalls.patch

In D32438#731940, @kib wrote:

Second, using O_PATH fd instead of normal fd does provide a delicate change of semantic. For instance, if you want take a bookmark for a file, for which you want to add or remove some extended attribute later, opening it for write now would prevent its execution. Opening it with O_PATH, and you can both execute and modify attributes, without loosing the file to a race with parallel rename or even ABA kind of substitution.

Yeah, the race condition stuff is why xdg-document-portal only holds O_PATH fds.

On Linux it does most operations with these fds by using the /proc/self/fd/$NUMBER path, which sort of sometimes works with our fdescfs's -o linrdlnk but seems dodgy. But since we have AT_EMPTY_PATH in lots more places, nearly everything can be done with the fds directly without going through any /dev/fd type paths. This extattr thing is one more place :)

This revision was automatically updated to reflect the committed changes.