From: Saurabh Singh Date: Thu, 3 Mar 2022 14:13:24 +0000 (+0530) Subject: Cleanup and bug fixes in vxfs vfs code. X-Git-Tag: talloc-2.4.0~1211 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2643b7b5746c7f9e8fba1aa7f4bb6fec47390842;p=thirdparty%2Fsamba.git Cleanup and bug fixes in vxfs vfs code. 1) Added debug messages in lib_vxfs.c for get, set and list attr functions 2) Removed vxfs_clearwxattr_fd and vxfs_clearwxattr_path code since it is no longer required now. 3) Replaced strcasecmp with vxfs_strcasecmp 4) Changed vxfs_fset_xattr to retain security.NTACL attribute 5) Fixed deny permissions not retained for a file created on CIFS share in vxfs_set_xattr Signed-off-by: Saurabh Singh Reviewed-by: Jeremy Allison Reviewed-by: Noel Power Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Sep 2 17:40:00 UTC 2022 on sn-devel-184 --- diff --git a/source3/modules/lib_vxfs.c b/source3/modules/lib_vxfs.c index 432eadd1865..e030cc36818 100644 --- a/source3/modules/lib_vxfs.c +++ b/source3/modules/lib_vxfs.c @@ -38,7 +38,6 @@ static int (*vxfs_getxattr_fd_func) (int fd, const char *name, void *value, static int (*vxfs_removexattr_fd_func) (int fd, const char *name); static int (*vxfs_listxattr_fd_func) (int fd, void *value, size_t *len); static int (*vxfs_setwxattr_fd_func) (int fd); -static int (*vxfs_clearwxattr_fd_func) (int fd); static int (*vxfs_checkwxattr_fd_func) (int fd); int vxfs_setxattr_fd(int fd, const char *name, const void *value, @@ -46,13 +45,16 @@ int vxfs_setxattr_fd(int fd, const char *name, const void *value, { int ret = -1; + DBG_DEBUG("In vxfs_setxattr_fd fd %d name %s len %zu flags %d\n", + fd, name, len, flags); if (vxfs_setxattr_fd_func == NULL) { errno = ENOSYS; return ret; } - DEBUG(10, ("Calling vxfs_setxattr_fd\n")); + DBG_DEBUG("Calling vxfs_setxattr_fd\n"); ret = vxfs_setxattr_fd_func(fd, name, value, len, flags); + DBG_DEBUG("vxfs_setxattr_fd ret = %d \n", ret); if (ret) { errno = ret; ret = -1; @@ -65,14 +67,17 @@ int vxfs_getxattr_fd(int fd, const char *name, void *value, size_t len) { int ret; size_t size = len; + DBG_DEBUG("In vxfs_getxattr_fd fd %d name %s len %zu\n", + fd, name, len); if (vxfs_getxattr_fd_func == NULL) { errno = ENOSYS; return -1; } - DEBUG(10, ("Calling vxfs_getxattr_fd with %s\n", name)); + DBG_DEBUG("Calling vxfs_getxattr_fd with %s\n", name); ret = vxfs_getxattr_fd_func(fd, name, value, &size); + DBG_DEBUG("vxfs_getxattr_fd ret = %d\n", ret); if (ret) { errno = ret; if (ret == EFBIG) { @@ -80,6 +85,7 @@ int vxfs_getxattr_fd(int fd, const char *name, void *value, size_t len) } return -1; } + DBG_DEBUG("vxfs_getxattr_fd done with size %zu\n", size); return size; } @@ -88,11 +94,13 @@ int vxfs_getxattr_path(const char *path, const char *name, void *value, size_t len) { int ret, fd = -1; + DBG_DEBUG("In vxfs_getxattr_path path %s name %s len %zu\n", + path, name, len); fd = open(path, O_RDONLY); if (fd == -1) { - DEBUG(10, ("file not opened: vxfs_getxattr_path for %s\n", - path)); + DBG_DEBUG("file not opened: vxfs_getxattr_path for %s\n", + path); return -1; } @@ -105,13 +113,14 @@ int vxfs_getxattr_path(const char *path, const char *name, void *value, int vxfs_removexattr_fd(int fd, const char *name) { int ret = 0; + DBG_DEBUG("In vxfs_removexattr_fd fd %d name %s\n", fd, name); if (vxfs_removexattr_fd_func == NULL) { errno = ENOSYS; return -1; } - DEBUG(10, ("Calling vxfs_removexattr_fd with %s\n", name)); + DBG_DEBUG("Calling vxfs_removexattr_fd with %s\n", name); ret = vxfs_removexattr_fd_func(fd, name); if (ret) { errno = ret; @@ -125,6 +134,7 @@ int vxfs_listxattr_fd(int fd, char *list, size_t size) { int ret; size_t len = size; + DBG_DEBUG("In vxfs_listxattr_fd fd %d list %s size %zu\n", fd, list, size); if (vxfs_listxattr_fd_func == NULL) { errno = ENOSYS; @@ -132,7 +142,8 @@ int vxfs_listxattr_fd(int fd, char *list, size_t size) } ret = vxfs_listxattr_fd_func(fd, list, &len); - DEBUG(10, ("vxfs_listxattr_fd: returned ret = %d\n", ret)); + DBG_DEBUG("vxfs_listxattr_fd: returned ret = %d\n", ret); + DBG_DEBUG("In vxfs_listxattr_fd done with len %zu\n", len); if (ret) { errno = ret; if (ret == EFBIG) { @@ -147,6 +158,7 @@ int vxfs_listxattr_fd(int fd, char *list, size_t size) int vxfs_setwxattr_fd(int fd) { int ret = 0; + DBG_DEBUG("In vxfs_setwxattr_fd fd %d\n", fd); if (vxfs_setwxattr_fd_func == NULL) { errno = ENOSYS; @@ -165,6 +177,7 @@ int vxfs_setwxattr_fd(int fd) int vxfs_setwxattr_path(const char *path, bool is_dir) { int ret, fd = -1; + DBG_DEBUG("In vxfs_setwxattr_path path %s is_dir %d\n", path, is_dir); if (is_dir) { fd = open(path, O_RDONLY|O_DIRECTORY); @@ -184,48 +197,10 @@ int vxfs_setwxattr_path(const char *path, bool is_dir) return ret; } -int vxfs_clearwxattr_fd(int fd) -{ - int ret; - if (vxfs_clearwxattr_fd_func == NULL) { - errno = ENOSYS; - return -1; - } - ret = vxfs_clearwxattr_fd_func(fd); - DBG_DEBUG("ret = %d\n", ret); - if (ret != 0) { - errno = ret; - ret = -1; - } - - return ret; -} - -int vxfs_clearwxattr_path(const char *path, bool is_dir) -{ - int ret, fd = -1; - - if (is_dir) { - fd = open(path, O_RDONLY|O_DIRECTORY); - } else { - fd = open(path, O_WRONLY); - } - - if (fd == -1) { - DBG_DEBUG("file %s not opened, errno:%s\n", - path, strerror(errno)); - return -1; - } - ret = vxfs_clearwxattr_fd(fd); - DBG_DEBUG("ret = %d\n", ret); - close(fd); - - return ret; -} - int vxfs_checkwxattr_fd(int fd) { int ret; + DBG_DEBUG("In vxfs_checkwxattr_fd fd %d\n", fd); if (vxfs_checkwxattr_fd_func == NULL) { errno = ENOSYS; @@ -243,6 +218,7 @@ int vxfs_checkwxattr_fd(int fd) int vxfs_checkwxattr_path(const char *path) { int ret, fd = -1; + DBG_DEBUG("In vxfs_checkwxattr_path path %s\n", path); fd = open(path, O_RDONLY); @@ -297,8 +273,6 @@ void vxfs_init() "vxfs_nxattr_list"); load_lib_vxfs_function(&lib_handle, &vxfs_setwxattr_fd_func, "vxfs_wattr_set"); - load_lib_vxfs_function(&lib_handle, &vxfs_clearwxattr_fd_func, - "vxfs_wattr_clear"); load_lib_vxfs_function(&lib_handle, &vxfs_checkwxattr_fd_func, "vxfs_wattr_check"); diff --git a/source3/modules/vfs_vxfs.c b/source3/modules/vfs_vxfs.c index ddd34ba812a..af9d3739948 100644 --- a/source3/modules/vfs_vxfs.c +++ b/source3/modules/vfs_vxfs.c @@ -62,8 +62,16 @@ along with this program. If not, see . * */ +/* + * Note: + * XATTR_USER_NTACL: This extended attribute is used + * to store Access Control List system objects by VxFS from DLV11 onwards. + * XATTR_USER_NTACL_V0: This extended attribute was used + * to store Access Control List system objects by VxFS till DLV10. + */ #ifndef XATTR_USER_NTACL -#define XATTR_USER_NTACL "system.NTACL" +#define XATTR_USER_NTACL "system.NTACL" +#define XATTR_USER_NTACL_V0 "user.NTACL" #endif /* type values */ @@ -75,6 +83,17 @@ along with this program. If not, see . #define VXFS_ACL_OTHER 5 #define VXFS_ACL_MASK 6 +/* + * Helper function for comparing two strings + */ +static int vxfs_strcasecmp(const char *str1, const char *str2) +{ + bool match = strequal_m(str1, str2); + if (match) { + return 0; + } + return 1; +} /* * Compare aces @@ -501,24 +520,36 @@ static int vxfs_fset_xattr(struct vfs_handle_struct *handle, struct files_struct *fsp, const char *name, const void *value, size_t size, int flags){ int ret = 0; + int tmp_ret = 0; - DEBUG(10, ("In vxfs_fset_xattr\n")); + DBG_DEBUG("In vxfs_fset_xattr\n"); ret = vxfs_setxattr_fd(fsp_get_io_fd(fsp), name, value, size, flags); if ((ret == 0) || ((ret == -1) && (errno != ENOTSUP) && (errno != ENOSYS))) { - SMB_VFS_NEXT_FREMOVEXATTR(handle, fsp, name); + /* + * version 1: user.NTACL xattr without inheritance supported + * version 2: user.NTACL xattr with inheritance supported + * version 3: new styled xattr security.NTACL with inheritance supported + * Hence, the old styled xattr user.NTACL should be removed + */ + tmp_ret = vxfs_strcasecmp(name, XATTR_NTACL_NAME); + if (tmp_ret == 0) { + SMB_VFS_NEXT_FREMOVEXATTR(handle, fsp, XATTR_USER_NTACL_V0); + DBG_DEBUG("Old style xattr %s removed...\n", XATTR_USER_NTACL_V0); + } + return ret; } - DEBUG(10, ("Fallback to xattr")); + DBG_DEBUG("Fallback to xattr"); if (strcmp(name, XATTR_NTACL_NAME) == 0) { return SMB_VFS_NEXT_FSETXATTR(handle, fsp, XATTR_USER_NTACL, value, size, flags); } /* Clients can't set XATTR_USER_NTACL directly. */ - if (strcasecmp(name, XATTR_USER_NTACL) == 0) { + if (vxfs_strcasecmp(name, XATTR_USER_NTACL) == 0) { errno = EACCES; return -1; } @@ -546,7 +577,7 @@ static ssize_t vxfs_fget_xattr(struct vfs_handle_struct *handle, } /* Clients can't see XATTR_USER_NTACL directly. */ - if (strcasecmp(name, XATTR_USER_NTACL) == 0) { + if (vxfs_strcasecmp(name, XATTR_USER_NTACL) == 0) { errno = ENOATTR; return -1; } @@ -566,7 +597,7 @@ static int vxfs_fremove_xattr(struct vfs_handle_struct *handle, XATTR_USER_NTACL); } else { /* Clients can't remove XATTR_USER_NTACL directly. */ - if (strcasecmp(name, XATTR_USER_NTACL) != 0) { + if (vxfs_strcasecmp(name, XATTR_USER_NTACL) != 0) { ret = SMB_VFS_NEXT_FREMOVEXATTR(handle, fsp, name); } @@ -595,7 +626,7 @@ static size_t vxfs_filter_list(char *list, size_t size) while (str - list < size) { size_t element_len = strlen(str) + 1; - if (strcasecmp(str, XATTR_USER_NTACL) == 0) { + if (vxfs_strcasecmp(str, XATTR_USER_NTACL) == 0) { memmove(str, str + element_len, size - (str - list) - element_len); @@ -636,10 +667,14 @@ static NTSTATUS vxfs_fset_ea_dos_attributes(struct vfs_handle_struct *handle, { NTSTATUS err; int ret = 0; - bool attrset = false; DBG_DEBUG("Entered function\n"); + err = SMB_VFS_NEXT_FSET_DOS_ATTRIBUTES(handle, fsp, dosmode); + if (!NT_STATUS_IS_OK(err)) { + DBG_DEBUG("err:%d\n", err); + return err; + } if (!(dosmode & FILE_ATTRIBUTE_READONLY)) { ret = vxfs_checkwxattr_fd(fsp_get_io_fd(fsp)); if (ret == -1) { @@ -656,21 +691,9 @@ static NTSTATUS vxfs_fset_ea_dos_attributes(struct vfs_handle_struct *handle, if ((errno != EOPNOTSUPP) && (errno != EINVAL)) { return map_nt_error_from_unix(errno); } - } else { - attrset = true; - } - } - err = SMB_VFS_NEXT_FSET_DOS_ATTRIBUTES(handle, fsp, dosmode); - if (!NT_STATUS_IS_OK(err)) { - if (attrset) { - ret = vxfs_clearwxattr_fd(fsp_get_io_fd(fsp)); - DBG_DEBUG("ret:%d\n", ret); - if ((ret == -1) && (errno != ENOENT)) { - return map_nt_error_from_unix(errno); - } } } - return err; + return NT_STATUS_OK; } static int vfs_vxfs_connect(struct vfs_handle_struct *handle, diff --git a/source3/modules/vfs_vxfs.h b/source3/modules/vfs_vxfs.h index afeccd85502..9194b8b256e 100644 --- a/source3/modules/vfs_vxfs.h +++ b/source3/modules/vfs_vxfs.h @@ -30,9 +30,6 @@ int vxfs_listxattr_fd(int, char *, size_t); int vxfs_setwxattr_path(const char *, bool); int vxfs_setwxattr_fd(int); -int vxfs_clearwxattr_path(const char *, bool); -int vxfs_clearwxattr_fd(int); - int vxfs_checkwxattr_path(const char *); int vxfs_checkwxattr_fd(int);