Page MenuHomeFreeBSD

Handle O_DIRECTORY on fdescfs files
Needs ReviewPublic

Authored by kib on May 5 2021, 8:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 9, 9:47 AM
Unknown Object (File)
Sun, Apr 21, 8:56 PM
Unknown Object (File)
Apr 13 2024, 1:28 AM
Unknown Object (File)
Apr 10 2024, 8:46 PM
Unknown Object (File)
Mar 31 2024, 2:13 PM
Unknown Object (File)
Mar 18 2024, 11:50 AM
Unknown Object (File)
Mar 10 2024, 11:41 PM
Unknown Object (File)
Feb 7 2024, 2:15 PM

Details

Reviewers
markj
emaste
Summary

If opened file with descriptor N references a directory, allow the open("/dev/fd/N", O_DIRECTORY) to succeed. To do it, postpone the check for v_type on fdescfs vnodes to fdesc_open() VOP, where check the type of real vnode instead of fdescfs node.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.May 5 2021, 8:15 PM
kib created this revision.
sys/fs/fdescfs/fdesc_vnops.c
426

You could get rid of error1 and several lines of code if error is initialized to 0 and the routine has return (error == 0 ? ENODEV : error).

sys/kern/vfs_vnops.c
405

We have vn_irflag_read(). In particular, the vnode interlock is not held here.

413

Don't we need to handle fdescfs dirs at least here as well?

kib marked 3 inline comments as done.May 12 2021, 8:16 PM

I fixed things you noted, but I am not sure if this patch is needed after O_EMPTY_PATH is added. I prefer to not add quirks for fdescfs if O_EMPTY_PATH is enough, since it both more clean and faster.

Lets wait for Andrew opinion.

sys/kern/vfs_vnops.c
405

VIRF_FDESCFS is stable after the vnode is visible, so I do not really see a need to use atomic_load() there, but ok.

413

I am not sure, and in fact I tend to answer 'no' to your question.

open("/dev/fd/N") is nothing more than dup(2). From this PoV, not returning error when O_DIRECTORY is specified and fd N references directory improves the semantic.

But we de-facto ignore open mode and O_TRUNC when opening fdescfs, so the fast that EISDIR is not returned is IMO fine. [Until somebody asks for this, for real porting need, of course]

kib marked 2 inline comments as done.

Use vn_irflag() KPI.
Simplify error tracking logic in fdescfs_open()