tmpfs silently ignore pathconf ACL requests

[sjg]

Summary

Emacs gets upset when copying file from fs with nfsv4acls
to tmpfs because fpathconf returns EINVAL.
This breaks mail handling with nmh.

[sjg]

fix patch skew

[kib]

I think that these cases can be usefully added to vop_stdpathconf() instead, to cover all filesystems. Just recheck that it does not break for UFS/ZFS/NFS.

[sjg]

shift to vop_stdpathconf

[kib]

You can remove handling of _PC_ACL_EXTENDED from nfs and zfs client vops.
You can remove any handling of _PC_ACL_EXTENDED and _PC_ACL_NFS4 from UFS wheh not compiled in (i.e. #else cases).

[jhb]

sys/kern/vfs_default.c

492

Looking at the manpage for pathconf, _PC_CAP_PRESENT, _PC_INF_PRESENT, and _PC_MAC_PRESENT look like good candidates for this as well.

[sjg]

Add more cases

[sjg]

In D19512#417632, @kib wrote:

You can remove handling of _PC_ACL_EXTENDED from nfs and zfs client vops.
You can remove any handling of _PC_ACL_EXTENDED and _PC_ACL_NFS4 from UFS wheh not compiled in (i.e. #else cases).

In the case of ufs this would get ugly no? The case would need to be within the #ifdef

[kib]

In D19512#417656, @sjg wrote:

In D19512#417632, @kib wrote:

You can remove handling of _PC_ACL_EXTENDED from nfs and zfs client vops.
You can remove any handling of _PC_ACL_EXTENDED and _PC_ACL_NFS4 from UFS wheh not compiled in (i.e. #else cases).

In the case of ufs this would get ugly no? The case would need to be within the #ifdef

No, as I said you drop #else part altogether.

[sjg]

In D19512#417662, @kib wrote:

In D19512#417656, @sjg wrote:

In D19512#417632, @kib wrote:

You can remove handling of _PC_ACL_EXTENDED from nfs and zfs client vops.
You can remove any handling of _PC_ACL_EXTENDED and _PC_ACL_NFS4 from UFS wheh not compiled in (i.e. #else cases).

In the case of ufs this would get ugly no? The case would need to be within the #ifdef

No, as I said you drop #else part altogether.

Just doing that won't work since default case would not be reached and thus vop_stdpathconf() would not be callled.
I can put the whole case _PC_ACL_EXTENDED and _PC_ACL_NFS4 inside #ifdef UFS_ACL

[sjg]

Like this?

[jhb]

In D19512#417671, @sjg wrote:

In D19512#417662, @kib wrote:

In D19512#417656, @sjg wrote:

In D19512#417632, @kib wrote:

You can remove handling of _PC_ACL_EXTENDED from nfs and zfs client vops.
You can remove any handling of _PC_ACL_EXTENDED and _PC_ACL_NFS4 from UFS wheh not compiled in (i.e. #else cases).

In the case of ufs this would get ugly no? The case would need to be within the #ifdef

No, as I said you drop #else part altogether.

Just doing that won't work since default case would not be reached and thus vop_stdpathconf() would not be callled.
I can put the whole case _PC_ACL_EXTENDED and _PC_ACL_NFS4 inside #ifdef UFS_ACL

Yes, I would put the entire case under the #ifdef and let it fall through the default if UFS_ACL isn't enabled. Something like

#ifdef UFS_ACL
      case _PC_ACL_EXTENDED:
          /* existing code that checks mount option */
          break;
#endif

[jhb]

The existing patch looks good to me (didn't see the UFS bits when I sent my last comment). Not sure if other filesystem pathconf methods need similar updates though? kib mentioned ZFS previously.

[kib]

Please handle NFS and ZFS.

sys/ufs/ufs/ufs_vnops.c

2437 ↗

(On Diff #54858)

You do need this empty line.

[sjg]

per feedback - not sure about zfs though

[kib]

In D19512#417682, @sjg wrote:

per feedback - not sure about zfs though

This looks good. For zfs, see

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c

In fact, grep the kernel sources for _PC_ACL_EXTENDED, you will find e.g. ext2fs and nandfs. I think you would prefer to keep ext2 as is, while nandfs does not need the _PC_ACL* cases after your changes.

[sjg]

Add zfs and nandfs, note zfs_vnops.c:zfs_pathconf is not compatible with this change

[sjg]

I applied this same patch to stable/11 and while ktace shows that tmpfs no longer returns EINVAL for fpathconf
original ktrace:

95279 emacs-26.1 CALL fpathconf(0x5,_PC_ACL_NFS4)
95279 emacs-26.1 RET fpathconf 1
95279 emacs-26.1 CALL acl_get_fd(0x5,ACL_TYPE_NFS4,0x8073ba000)
95279 emacs-26.1 RET acl_get_fd 0
95279 emacs-26.1 CALL fpathconf(0x8,_PC_ACL_NFS4)
95279 emacs-26.1 RET fpathconf -1 errno 22 Invalid argument

new ktrace:

77748 emacs-26.1 CALL fpathconf(0x5,_PC_ACL_NFS4)
77748 emacs-26.1 RET fpathconf 1
77748 emacs-26.1 CALL acl_get_fd(0x5,ACL_TYPE_NFS4,0x804e34000)
77748 emacs-26.1 RET acl_get_fd 0
77748 emacs-26.1 CALL fpathconf(0x6,_PC_ACL_NFS4)
77748 emacs-26.1 RET fpathconf 0

emacs is still unhappy - at this point it is an emacs bug I think.
I'm *guessing* that the different returns from fpathconf upset it - which is silly?
The ktrace shows no other issue to complain about.

Regardless the above constitutes a successful test of this change ;-)