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 <saurabh.singh@veritas.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Noel Power <noel.power@suse.com>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
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 432eadd1865007b3c155963c692877adb0d631f4..e030cc368181d76b87959549bb05a8446282fa4c 100644 (file)
--- 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 ddd34ba812a0401d1133eec543297a893c093c1e..af9d37399480bf2d1be53f8c14a660295ced1caf 100644 (file)
--- a/source3/modules/vfs_vxfs.c
+++ b/source3/modules/vfs_vxfs.c
@@ -62,8 +62,16 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
 
+/*
+ * 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 <http://www.gnu.org/licenses/>.
 #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 afeccd85502f7686315e327238604cd207819023..9194b8b256e3905150089020607ffb495c58ae4c 100644 (file)
--- 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);