Page MenuHomeFreeBSD

Various pathconf() fixes.
ClosedPublic

Authored by jhb on Oct 3 2017, 12:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
May 24 2024, 8:39 PM
Unknown Object (File)
Dec 20 2023, 3:22 AM
Unknown Object (File)
Dec 17 2023, 2:08 PM
Unknown Object (File)
Dec 14 2023, 10:54 PM
Unknown Object (File)
Oct 19 2023, 9:21 PM
Unknown Object (File)
Sep 23 2023, 5:18 PM
Unknown Object (File)
Sep 3 2023, 6:25 AM
Unknown Object (File)
Aug 4 2023, 7:27 PM
Subscribers

Details

Summary

I will not commit this as a giant patch but will probably split it up
(e.g. handling LINK_MAX for individual filesystems as commits, probably
a single commit to move various settings out of vop_stdpathconf(), and
probably a separate commit for _PC_PIPE_BUF and switching FIFOs to use
their native fs' pathconf method). However, I think it can probably be
reviewed as one big chunk where one can see the cumulative effects.

Log messages from commits on this branch so far:

Add a real VOP_PATHCONF for fdescfs.

Honor the hardcoded link counts when reporting the LINK_MAX attribute.
Forward requests for variables other than NAME_MAX and LINK_MAX to the
underlying file descriptor via kern_fpathconf().

Flesh out ZFS pathconf handling.

  • Handle _PC_NAME_MAX, _PC_PIPE_BUF (only dirs and fifos), and _PC_CHOWN_RESTRICTED.
  • Use regular pathconf method for FIFOs. In particular, this supports _PC_NAME_MAX, _PC_LINK_MAX and several other variables for FIFOs.

Flesh out devfs_pathconf some more.

  • _PC_FILESIZEBITS, _PC_NAME_MAX, _PC_LINK_MAX, _PC_SYMLINK_MAX, _PC_CHOWN_RESTRICTED.

ext2 pathconf fixes

  • Use ext2_pathconf for FIFOs.
  • Handle _PC_NAME_MAX, _PC_PIPE_BUF, and _PC_CHOWN_RESTRICTED.

Add a real implementation of VOP_PATHCONF for fuse.

This doesn't set _PC_CHOWN_RESTRICTED because I couldn't tell if chown
operations are actually restricted. fuse's setattr VOP sets a FACCESS_CHOWN
flag only to clear it via the FACCESS_XQUERIES mask before calling
fuse_internal_access(). For its part, fuse_internal_access() doesn't check
FACCESS_CHOWN. Perhaps a filesystem can optionally choose to enforce
restrictions, but it would need to be unconditional to set _PC_CHOWN_RESTRICTED
I think.

msdosfs pathconf fixes.

  • Put back _PC_CHOWN_RESTRICTED.
  • Add _PC_FILESIZEBITS.

nandfs pathconf fixes.

  • Bring back _PC_LINK_MAX (but NANDFS_LINK_MAX), _PC_NAME_MAX (but NANDFS_NAME_LEN), _PC_PIPE_BUF (only dirs and fifos), and _PC_CHOWN_RESTRICTED.
  • Use nandfs_pathconf for FIFOs.
  • While here, consistently use NANDFS_LINK_MAX instead of LINK_MAX.

Use nlink_t instead of u_short for link count type.

This avoids truncating link counts > 2^15.

NFS pathconf fixes.

  • Use INT_MAX as fallback link max (RPC field is 32 bits)
  • Bring back _PC_PIPE_BUF.
  • Use nfs_pathconf for FIFOs.

smbfs pathconf fixes.

  • Remove _PC_LINK_MAX rather than returning 0.
  • Handle _PC_FILESIZEBITS.
  • Claim _PC_NO_TRUNC.

tmpfs pathconf fixes.

Note that FIFOs don't actually work since they don't use a separate VOP
vector.

udf pathconf fixes

  • Use ufd_pathconf for FIFOs.
  • Handle _PC_PIPE_BUF.

UFS pathconf fixes.

  • Use ufs_pathconf for FIFOs.
  • Add UFS_LINK_MAX and use instead of LINK_MAX.
  • Restore _PC_LINK_MAX, _PC_PIPE_BUF, and _PC_CHOWN_RESTRICTED.

Retire FIFO pathconf method.

Trim vop_stdpathconf.

Drop _PC_NAME_MAX, _PC_LINK_MAX, _PC_PIPE_BUF, _PC_CHOWN_RESTRICTED.

Use tmpfs_pathconf for FIFOs on tmpfs.

Export tmpfs_pathconf.

More LINK_MAX fixes for ZFS.

In particular, add a ZFS_LINK_MAX which is 2^64 - 1. Use it to re-enable
EMLINK checking in zfs_link_create() and avoid capping va_nlink to LINK_MAX
as that is no longer needed.

Add an NFS_LINK_MAX and replace LINK_MAX usage with it.

I'm not certain ZFS really does CHOWN_RESTRICTED.

Use uint32_t cast to avoid sign extension.

Use 'int' for tmpfs link count and adopt a TMPFS_LINK_MAX.

Use NFS_LINK_MAX.

Truncate to LONG_MAX due to pathconf() API limitations.

Clean up after rebase of tmpfs_print commit.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h
204 ↗(On Diff #33645)

Perhaps bracket this with ifdef __FreeBSD__ ?
Is this constant going to be signed or unsigned? Maybe make that explicit?
Also, can it be defined in terms of some other constant, so that one does not have to count 'f'-s? :-)

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
2713 ↗(On Diff #33645)

Looks like this can potentially overflow if z_links is already at maximum?

4420 ↗(On Diff #33645)

Should this be LONG_MAX or ULONG_MAX?
Pardon my ignorance.

This revision is now accepted and ready to land.Oct 3 2017, 11:11 AM

NFS part looks ok to me. (I might have used UINT32_MAX instead of 0xffffffff for NFS_MAX_LINK, but
that's a very minor nit..)

For NFSv3, the link count is uint32_t and then the actual limit it returned by the PATHCONF RPC
(for NFSv4 the same happens, except that it is a Getattr Op), so I think the correct limit should
normally be returned at the client end with this patch applied. (I'll double check that the server side doesn't somehow
truncate it to a u_short anywhere.)

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
530 ↗(On Diff #33645)

BTW, I had assumed the #if 0 was because this was present in illumos but disabled in FreeBSD due to nlink_t being so short before ino64. However, I can't find any references to link checks in illumos. Apparently ZFS on illumos will happily allow the link to wrap around to 0. :( So, not sure if we should enable this code?

541 ↗(On Diff #33645)

This condition probably needs to be:

if (zp->z_links + zp_is_dir >= ZFS_LINK_MAX)

? Not sure if zp_is_dir can be true without ZRENAMING being set.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
2713 ↗(On Diff #33645)

Yes, I had considered maybe making ZFS_LINK_MAX 'N - 1' to avoid that condition (for some value of 'N')

4420 ↗(On Diff #33645)

The pathconf() family of system calls returns 'long' per POSIX. Using ULONG_MAX (as illumos does) means it would actually return a value of -1. Returning -1 from pathconf() without setting errno means that there is no limit for the value (in this case for {LINK_MAX}). That is definitely not true.

We could just use ZFS_LINK_MAX as LONG_MAX (or LONG_MAX - 1) which would permit 2^31-1 links on 32-bit systems and 2^63-1 on 64-bit systems. That is probably enough and would avoid the need for this MIN() perhaps?

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
530 ↗(On Diff #33645)

That's an interesting observation. Indeed, it seems that the code was added to prevent the link count going over the [old] FreeBSD imposed limits. I am not sure why it was never enabled, perhaps it caused some problems.

Regarding illumos, maybe their thinking is that with an unsigned 64-bit link count you would run out of other resources before being able to hit the maximum here? E.g., object / file IDs are also of the same type, and some of them are reserved. So, we simply can not create enough files to overflow the link count.

So...

541 ↗(On Diff #33645)

I think that it can also be true for mkdir.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
2713 ↗(On Diff #33645)

That would help...
But now I think that we probably don't need to worry much about this given that z_links can not really approach its maximum value.

4420 ↗(On Diff #33645)

I am okay with the code as it is now. I think it's not a problem if we advertise a smaller number here while internally supporting a bigger number.

This revision was automatically updated to reflect the committed changes.