Add VFS/syscall support for Solaris style extended attributes (called named attributes by NFSv4)

[rmacklem]

Summary

Some systems, such as Solaris, represent extended attributes as
a set of files in a directory associated with a file object. This
allows extended attributes to be acquired/modified via regular
file system operations, such as read(2), write(2), lseek(2) and ftruncate(2).

Since ZFS already has the capability to do this, this patch allows system
calls (and the NFSv4 client/server) such access to extended attributes.
This permits handling of large extended attributes and allows the NFSv4
server to provide the service to NFSv4 clients that want it, such as Windows,
MacOS and Solaris.

The top level syscall change is a new open(2)/openat(2) flag I called O_NAMEDATTR
that allows the named attribute directory or any attribute within that directory to
be open'd.

The patch defines two new v_irflag flags called VIRF_NAMEDDIR and VIRF_NAMEDATTR
to indicate that the vnode is for this alternate name space and not a normal file object.
The patch also defines flags (OPENNAMED and CREATENAMED) for VOP_LOOKUP()
to pass this new case down into VOP_LOOKUP().

Most of the code in this patch is to avoid creation of links, symlinks or non-regular
file objects in the named attribute directory.

It also must avoid using the name cache, since the named attribute directory is
associated with the same name as the file object. (I am not so sure w.r.t. the
attribute names in the named attribute directory, but felt it was easier and safe
to avoid name caching there, as well.)

If you'd like to see it, I can put the ZFS patch in phabricator, as well?

This has been discussed recently on freebsd-current@ under
RFC: Solaris style extended attributes for FreeBSD
and on freebsd-hackers@ under
FreeBSD NFSv4.1 nfsd, named attribute support (OPENATTR)?

Test Plan

I have been doing a modicum of testing using a couple
of simple test programs, which can be found at:
https://people.freebsd.org/~rmacklem/xattrtest.c
https://people.freebsd.org/~rmacklem/xattrtest1.c
and it seems to work ok for a patched ZFS system,
where ZFS's xattr property is set to on/dir.

[kib]

Do we need to filter the fhopen(2) and other handle-related syscalls?

sys/sys/mount.h

520

Why put the flag into MNTK_ space, and not MNT_? I think we want to inform userspace about named attrs support for specific mp.

sys/sys/namei.h

182

Why the flag is needed, as opposed to the op == CREATE and cn_flags OPENNAMED?

[rmacklem]

I need to think about fhopen(2) and friends.
I thought I had disabled use of O_NAMEDATTR
for them, but maybe I missed that in the patch.

The problem is that fhopen(2) and friends is
mainly useful for userspace NFS servers and
the named attribute directory plus attribute files
in it are different File handle types (not dir and reg).
So, the userspace NFS server would need to know
these file handles are named attribute related or not?

I'll think about this one.

sys/sys/mount.h

520

I guess I was thinking that MNT_xxx are related to
mount options and not properties of the file system.

But if you think MNT_NAMEDATTR is more appropriate,
I'll change it. (For ZFS, it is controlled by the "xattr" property.)

sys/sys/namei.h

182

Well op == CREATE has assorted implications and
since CREATENAMED is only a modifier for OPENNAMED
to say "create the named attribute directory if it does not
exist". It does not imply create the file object unless it is
a openat(2) with the fd that represents a named attribute dir.

For example:

fd = open("foo", O_RDONLY | O_CREAT | O_NAMED, 0600);

doesn't mean "create "foo". It means lookup foo and if it
exists, then create a named attribute directory for it, if it
does not exist. Reply ENOENT if "foo" does not exist.

I felt having op == CREATE would be too confusing for this
case and I think there are at least some KASSERT()s that
might blow up, etc.

[rmacklem]

W.r.t. fhopen(2), I think there is a couple of ways to handle
O_NAMEDATTR.

• Just don't allow it, similar to O_CREAT.

or

• Fail the fhopen(2) if the file handle does not represent a named attribute directory or a named attribute file.

Do you have a preference?

I need to test getfhat(2) to make sure it can get a named attribute
directory by using a path of "." or a named attribute file for other
names. (They need to be filtered so that no "/", is in the oath.)

sys/sys/namei.h

182

I meant O_NAMEDATTR in the open(2) example
and not O_NAMED. A typo that others might find confusing, sorry.

[rmacklem]

Fyi, I did a little testing of fhopen(2) and getfhat(2) and
everything seems to work.

The only quirk is that O_NAMEDATTR as an argument flag
for fhopen(2) is ignored. This should probably change as I
noted before.

Maybe...

• O_NAMEDATTR returns EINVAL

or

• O_NAMEDATTR checks to see that the fh being open'd is a named attribute or named attribute directory and replies ENOATTR if it is not one of these.

Which of these sounds preferable?

[rmacklem]

Changed MNTK_NAMEDATTR to MNT_NAMEDATTR as
suggested by kib@.

Added a check for the O_NAMEDATTR flag for fhopen(2),
since the flag is meaningless. The fh might represent a
named attribute directory or named attribute, but there
is no need for the flag. (getfhat(2) can be used to get a
file handle for a named attribute or named attribute directory.)
I think that is all that is needed for fhopen(2) and friends.

Added an entry for _PC_NAMEDATTR_ENABLED to vop_stdpathconf().

Updated PARAMASK to include CREATENAMED.

I am uncomfortable using cn_nameiop == CREATE for creation of
the named attribute directory, since it is not the file object represented
by the path (which is the parent file object).

[kib]

I think fhopen() should require O_NAMEDATTR to successfully open the attribute inode' handle.
BTW, given a file descriptor, is there a way to understand if this is a regular fd or namedattr fd? In particular, does fstatat(2) return some distinguishing info?

[rmacklem]

In D49583#1131379, @kib wrote:

I think fhopen() should require O_NAMEDATTR to successfully open the attribute inode' handle.
BTW, given a file descriptor, is there a way to understand if this is a regular fd or namedattr fd? In particular, does fstatat(2) return some distinguishing info?

Not a fd. The code needs to get the associated vnode and look for VIRF_NAMEDDIR or VIRF_NAMEDATTR being set.
(I'll tweak fhopen(2) to do that.)

No, I don't have fstatat(2) returning anything in "struct stat" that indicates "namedattr".
(st_flags seem to be all bits that can be set/cleared by chflags and I cannot see any
other field of "struct stat" that could be used to indicate a named attribute dir/attribute.)
Any suggestions?

[kib]

In D49583#1131391, @rmacklem wrote:

In D49583#1131379, @kib wrote:

I think fhopen() should require O_NAMEDATTR to successfully open the attribute inode' handle.
BTW, given a file descriptor, is there a way to understand if this is a regular fd or namedattr fd? In particular, does fstatat(2) return some distinguishing info?

Not a fd. The code needs to get the associated vnode and look for VIRF_NAMEDDIR or VIRF_NAMEDATTR being set.
(I'll tweak fhopen(2) to do that.)

No, I don't have fstatat(2) returning anything in "struct stat" that indicates "namedattr".
(st_flags seem to be all bits that can be set/cleared by chflags and I cannot see any
other field of "struct stat" that could be used to indicate a named attribute dir/attribute.)
Any suggestions?

The regular way would be to define new S_IFMT value for named attr files, but this probably would break too much.
We have st_padding0 field that can be used for additional flags, like st_bsd_flags or similar, and we can allocate the first bit there to mean named attr file. The rename of st_padding0 and the flag definition should be a separate commit.

[rmacklem]

Change fhopen(2) so that O_NAMEDATTR is required to
open a file handle that represents a named attribute or
named attribute directory.

If O_NAMEDATTR is specified and the file handle does not
represent a named attribute or named attribute directory,
it fails as well.

Both failures return ENOATTR.

[rmacklem]

In D49583#1131392, @kib wrote:

In D49583#1131391, @rmacklem wrote:

In D49583#1131379, @kib wrote:

I think fhopen() should require O_NAMEDATTR to successfully open the attribute inode' handle.
BTW, given a file descriptor, is there a way to understand if this is a regular fd or namedattr fd? In particular, does fstatat(2) return some distinguishing info?

Not a fd. The code needs to get the associated vnode and look for VIRF_NAMEDDIR or VIRF_NAMEDATTR being set.
(I'll tweak fhopen(2) to do that.)

No, I don't have fstatat(2) returning anything in "struct stat" that indicates "namedattr".
(st_flags seem to be all bits that can be set/cleared by chflags and I cannot see any
other field of "struct stat" that could be used to indicate a named attribute dir/attribute.)
Any suggestions?

The regular way would be to define new S_IFMT value for named attr files, but this probably would break too much.
We have st_padding0 field that can be used for additional flags, like st_bsd_flags or similar, and we can allocate the first bit there to mean named attr file. The rename of st_padding0 and the flag definition should be a separate commit.

Sounds like a good plan to me. Early on, I decided to avoid new vnode types for the same reason.
And there are directories and files, just outside the file system tree's namespace.

[kib]

Do you want me to prepare the generic patch for stat(2)? Then you would add the setting of the bits as the followup.

sys/sys/mount.h

434

Please also add the flag to statfs.2 man page.

sys/sys/namei.h

183

I think this is fine, but also could be 0x9ffffe00 ?

[rmacklem]

In D49583#1131634, @kib wrote:

Do you want me to prepare the generic patch for stat(2)? Then you would add the setting of the bits as the followup.

Sure, that would be great if you find the time. Otherwise, I will do it someday.

sys/sys/mount.h

434

Yes. I need to do patches for a few man pages.
(open.2, fhopen.2 and some new one for named attributes)
I'll add this to the list.

sys/kern/vfs_cache.c

cache_fpl_terminated(struct cache_fpl *fpl)
 
         return (fpl->status != CACHE_FPL_STATUS_UNSET);
 }
 
 #define CACHE_FPL_SUPPORTED_CN_FLAGS \
         (NC_NOMAKEENTRY | NC_KEEPPOSENTRY | LOCKLEAF | LOCKPARENT | WANTPARENT | \
          FAILIFEXISTS | FOLLOW | EMPTYPATH | LOCKSHARED | ISRESTARTED | WILLBEDIR | \
          ISOPEN | NOMACCHECK | AUDITVNODE1 | AUDITVNODE2 | NOCAPCHECK | OPENREAD | \
-         OPENWRITE | WANTIOCTLCAPS)
+         OPENWRITE | WANTIOCTLCAPS | OPENNAMED)
 
 #define CACHE_FPL_INTERNAL_CN_FLAGS \
         (ISDOTDOT | MAKEENTRY | ISLASTCN)
 
 _Static_assert((CACHE_FPL_SUPPORTED_CN_FLAGS & CACHE_FPL_INTERNAL_CN_FLAGS) == 0,
     "supported and internal flags overlap");
 
 static bool
if (IN_CAPABILITY_MODE(td) || CAP_TRACING(td)) {
                 cache_fpl_aborted_early(fpl);
                 return (false);
         }
         if (AUDITING_TD(td)) {
                 cache_fpl_aborted_early(fpl);
                 return (false);
         }
         if (ndp->ni_startdir != NULL) {
+                cache_fpl_aborted_early(fpl);
+                return (false);
+        }
+        if ((cnp->cn_flags & OPENNAMED) != 0) {
                 cache_fpl_aborted_early(fpl);
                 return (false);
         }
         return (true);
 }
 
 static int __noinline
 cache_fplookup_dirfd(struct cache_fpl *fpl, struct vnode **vpp)

sys/kern/vfs_default.c

case _PC_PATH_MAX:
                         *ap->a_retval = PATH_MAX;
                         return (0);
                 case _PC_ACL_EXTENDED:
                 case _PC_ACL_NFS4:
                 case _PC_CAP_PRESENT:
                 case _PC_DEALLOC_PRESENT:
                 case _PC_INF_PRESENT:
                 case _PC_MAC_PRESENT:
+                case _PC_NAMEDATTR_ENABLED:
                         *ap->a_retval = 0;
                         return (0);
                 default:
                         return (EINVAL);
         }
         /* NOTREACHED */
 }
 

sys/kern/vfs_lookup.c

if (ndp->ni_startdir != NULL) {
                         if (cnp->cn_flags & AUDITVNODE1)
                                 AUDIT_ARG_ATFD1(ndp->ni_dirfd);
                         if (cnp->cn_flags & AUDITVNODE2)
                                 AUDIT_ARG_ATFD2(ndp->ni_dirfd);
 
                         error = fgetvp_lookup(ndp, dpp);
                 }
                 if (error == 0 && (*dpp)->v_type != VDIR &&
+                    (cnp->cn_flags & OPENNAMED) == 0 &&
                     (cnp->cn_pnbuf[0] != '\0' ||
                     (cnp->cn_flags & EMPTYPATH) == 0))
                         error = ENOTDIR;
         }
         if (error == 0 && (cnp->cn_flags & RBENEATH) != 0) {
                 if (cnp->cn_pnbuf[0] == '/') {
                         error = ENOTCAPABLE;
                 } else if ((ndp->ni_lcf & NI_LCF_STRICTREL) == 0) {

sys/kern/vfs_subr.c

#define MNT_FLAG(flag)  do {                                            \
         MNT_FLAG(MNT_USER);
         MNT_FLAG(MNT_IGNORE);
         MNT_FLAG(MNT_UPDATE);
         MNT_FLAG(MNT_DELEXPORT);
         MNT_FLAG(MNT_RELOAD);
         MNT_FLAG(MNT_FORCE);
         MNT_FLAG(MNT_SNAPSHOT);
         MNT_FLAG(MNT_BYFSID);
+        MNT_FLAG(MNT_NAMEDATTR);
 #undef MNT_FLAG
         if (mflags != 0) {
                 if (buf[0] != '\0')
                         strlcat(buf, ", ", sizeof(buf));
                 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
                     "0x%016jx", mflags);
         }
         db_printf("    mnt_flag = %s\n", buf);