If using "vfs object" with "zfsacl" on FreeBSD 13.2 I get: smb: \> mkdir qqq NT_STATUS_INVALID_HANDLE making remote directory \qqq If I up the "log level" to 10 then the same operation gives in the log file: [2023/05/25 12:12:49.580439, 5, pid=82150, effective(1003258, 100001000), real(0, 0)] ../../source3/smbd/dosmode.c:181(unix_mode) unix_mode: unix_mode(qqq) returning 040700 facl(ACE_GETACLCNT, .): Bad file descriptor check_parent_access_fsp: SMB_VFS_FGET_NT_ACL failed for . with error NT_STATUS_INVALID_HANDLE [2023/05/25 12:12:49.580490, 5, pid=82150, effective(1003258, 100001000), real(0, 0)] ../../source3/smbd/open.c:4571(mkdir_internal) mkdir_internal: check_parent_access_fsp on directory . for path qqq returned NT_STATUS_INVALID_HANDLE (Same problem with Samba 4.17.6, 4.17.8 & 4.18.2) Works on FreeBSD 12.3/12.4
This seems to happen not just for MKDIR, but almost all operations working on files. "DIR" and "CD" works fine.
Ah, I think I found it. It's the O_PATH support that is new in FreeBSD 13 that doesn't play well with the ACL functions - acl_get_fd_np() doesn't work for an O_PATH opened file. Test code that fails: include <stdio.h> #include <stdlib.h> #include <string.h> #include <errno.h> #include <fcntl.h> #include <sys/types.h> #include <sys/acl.h> #include <sys/stat.h> int main(int argc, char *argv[]) { int i, j, fd; acl_t a; char *path = "."; int flags = O_PATH; struct stat sb; for (i = 1; i < argc && argv[i][0] == '-'; i++) { for (j = 1; argv[i][j]; j++) { switch (argv[i][j]) { case 'n': flags = 0; break; } } } if (i < argc) path = argv[i]; fd = open(path, O_RDONLY|flags); if (fd < 0) { fprintf(stderr, "%s: Error: %s: open: %s\n", argv[0], path, strerror(errno)); exit(1); } if (fstat(fd, &sb) < 0) { fprintf(stderr, "%s: Error: %s: fstat: %s\n", argv[0], path, strerror(errno)); } a = acl_get_fd_np(fd, ACL_TYPE_NFS4); if (!a) { fprintf(stderr, "%s: Error: %s: acl_get_fd_np: %s\n", argv[0], path, strerror(errno)); } return 0; } Lpeter86@runur00:~ % ./t ./t: Error: .: acl_get_fd_np: Bad file descriptor Lpeter86@runur00:~ % ./t -n
A workaround: 1. Edit /usr/include/fcntl.h and rename O_PATH to DISABLED_O_PATH 2. Build Samba 3. Undo the change to fcntl.h ...
Created attachment 17900 [details] Patch the only enables Samba O_PATH support on Linux systems Attachment contains a patch that only enables the O_PATH support for Linux systems. Works but is kind of a big hammer...
Here is the code inside posixacl_sys_acl_get_fd() that does this for POSIX ACLs. if (!fsp->fsp_flags.is_pathref && (acl_type == ACL_TYPE_ACCESS)) { /* POSIX API only allows ACL_TYPE_ACCESS fetched on fd. */ acl = acl_get_fd(fsp_get_io_fd(fsp)); } else if (fsp->fsp_flags.have_proc_fds) { int fd = fsp_get_pathref_fd(fsp); const char *proc_fd_path = NULL; char buf[PATH_MAX]; proc_fd_path = sys_proc_fd_path(fd, buf, sizeof(buf)); if (proc_fd_path == NULL) { return NULL; } acl = acl_get_file(proc_fd_path, acl_type); fsp->fsp_flags.is_pathref = true for fd's open with O_PATH. I think zfsacl may need something similar for FreeBSD.
These are the current places where the horrid hack is used: git grep sys_proc_fd_path source3/include/proto.h:const char *sys_proc_fd_path(int fd, char *buf, size_t bufsize); source3/lib/system.c:const char *sys_proc_fd_path(int fd, char *buf, size_t bufsize) source3/modules/vfs_btrfs.c: p = sys_proc_fd_path(fsp_fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_default.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_gpfs.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_gpfs.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_gpfs.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_gpfs.c: p = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_posixacl.c: proc_fd_path = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_posixacl.c: proc_fd_path = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/modules/vfs_posixacl.c: proc_fd_path = sys_proc_fd_path(fd, buf, sizeof(buf)); source3/smbd/open.c: p = sys_proc_fd_path(old_fd, buf, sizeof(buf)); So these are all the places you're going to run into the same problem with O_PATH opened fd's.
Created attachment 17901 [details] Patch that implements openat() O_PATH fix for FreeBSD instead of the proc_fds hack Here is a first cut at a patch that uses openat() to get a usable fd handle for systems that support it (FreeBSD) in order to avoid the fallback to using stored paths in smbd/open.c, modules/vfs_default & modules/vfs_zfsacl.
Comment on attachment 17901 [details] Patch that implements openat() O_PATH fix for FreeBSD instead of the proc_fds hack It works, but I hate the #ifdef wrapping :-). How about the: #if defined(HAVE_OPENAT) && defined(O_EMPTY_PATH) new_fd = openat(old_fd, "", O_EMPTY_PATH|flags); ... #endif construct is wrapped into an inline function, something like: ------------------------------------------------------ inline int open_metadata_fd_from_pathref(int md_fd) { #if defined(HAVE_OPENAT) && defined(O_EMPTY_PATH) return openat(md_fd, "", O_EMPTY_PATH|flags); #else return -1; } ----------------------------------------------------- That should allow open_metadata_fd_from_pathref() to always be called, with the logic from a success return always included, with the fsp->fsp_flags.have_proc_fds as the first fallback.
Yeah I missed the 'int flags' arg, but I'm sure you get what I mean.
Or maybe not return -1 from the open_metadata_fd_from_pathref() call, as I guess that could be a valid error return if the openat(.. O_EMPTY_PATH) fails. So maybe have it look like: inline int open_metadata_fd_from_pathref(int md_fd, int flags. int *_new_fd) { #if defined(HAVE_OPENAT) && defined(O_EMPTY_PATH) int new_fd new_fd = openat(old_fd, "", O_EMPTY_PATH|flags); if (new_fd == -1) { return errno; } *_new_fd = new_fd; return 0; #else return ENOSYS; #endif }