On Sat, 2018-07-14 at 16:06 -0300, Ernesto A. Fernández wrote:
> The HFS+ Access Control Lists have not worked at all for the past
> five
> years, and nobody seems to have noticed. Besides, POSIX draft ACLs
> are
> not compatible with MacOS. Drop the feature entirely.
>
Bugs need to be fixed but not to drop. Otherwise, it needs to drop the
whole HFS+ support from the kernel.
Thanks,
Vyacheslav Dubeyko.
> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@xxxxxxxxx>
> Acked-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/hfsplus/Kconfig | 15 -----
> fs/hfsplus/Makefile | 2 -
> fs/hfsplus/acl.h | 28 ---------
> fs/hfsplus/dir.c | 9 +--
> fs/hfsplus/hfsplus_fs.h | 1 -
> fs/hfsplus/inode.c | 11 ----
> fs/hfsplus/posix_acl.c | 144 ----------------------------------
> ----------
> fs/hfsplus/super.c | 4 +-
> fs/hfsplus/xattr.c | 6 --
> fs/hfsplus/xattr.h | 3 -
> fs/hfsplus/xattr_security.c | 13 ----
> 11 files changed, 4 insertions(+), 232 deletions(-)
> delete mode 100644 fs/hfsplus/acl.h
> delete mode 100644 fs/hfsplus/posix_acl.c
>
> diff --git a/fs/hfsplus/Kconfig b/fs/hfsplus/Kconfig
> index 7cc8b4acf66a..a63371815aab 100644
> --- a/fs/hfsplus/Kconfig
> +++ b/fs/hfsplus/Kconfig
> @@ -11,18 +11,3 @@ config HFSPLUS_FS
> MacOS 8. It includes all Mac specific filesystem data such
> as
> data forks and creator codes, but it also has several UNIX
> style features such as file ownership and permissions.
> -
> -config HFSPLUS_FS_POSIX_ACL
> - bool "HFS+ POSIX Access Control Lists"
> - depends on HFSPLUS_FS
> - select FS_POSIX_ACL
> - help
> - POSIX Access Control Lists (ACLs) support permissions for
> users and
> - groups beyond the owner/group/world scheme.
> -
> - It needs to understand that POSIX ACLs are treated only
> under
> - Linux. POSIX ACLs doesn't mean something under Mac OS X.
> - Mac OS X beginning with version 10.4 ("Tiger") support
> NFSv4 ACLs,
> - which are part of the NFSv4 standard.
> -
> - If you don't know what Access Control Lists are, say N
> diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
> index f6a56542f8d7..9ed20e64b983 100644
> --- a/fs/hfsplus/Makefile
> +++ b/fs/hfsplus/Makefile
> @@ -8,5 +8,3 @@ obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o
> hfsplus-objs := super.o options.o inode.o ioctl.o extents.o
> catalog.o dir.o btree.o \
> bnode.o brec.o bfind.o tables.o unicode.o wrapper.o
> bitmap.o part_tbl.o \
> attributes.o xattr.o xattr_user.o xattr_security.o
> xattr_trusted.o
> -
> -hfsplus-$(CONFIG_HFSPLUS_FS_POSIX_ACL) += posix_acl.o
> diff --git a/fs/hfsplus/acl.h b/fs/hfsplus/acl.h
> deleted file mode 100644
> index 488c2b75cf41..000000000000
> --- a/fs/hfsplus/acl.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * linux/fs/hfsplus/acl.h
> - *
> - * Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> - *
> - * Handler for Posix Access Control Lists (ACLs) support.
> - */
> -
> -#include <linux/posix_acl_xattr.h>
> -
> -#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
> -
> -/* posix_acl.c */
> -struct posix_acl *hfsplus_get_posix_acl(struct inode *inode, int
> type);
> -int hfsplus_set_posix_acl(struct inode *inode, struct posix_acl
> *acl,
> - int type);
> -extern int hfsplus_init_posix_acl(struct inode *, struct inode *);
> -
> -#else /* CONFIG_HFSPLUS_FS_POSIX_ACL */
> -#define hfsplus_get_posix_acl NULL
> -#define hfsplus_set_posix_acl NULL
> -
> -static inline int hfsplus_init_posix_acl(struct inode *inode, struct
> inode *dir)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_HFSPLUS_FS_POSIX_ACL */
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index b5254378f011..c5a70f83dbe7 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -18,7 +18,6 @@
> #include "hfsplus_fs.h"
> #include "hfsplus_raw.h"
> #include "xattr.h"
> -#include "acl.h"
>
> static inline void hfsplus_instantiate(struct dentry *dentry,
> struct inode *inode, u32
> cnid)
> @@ -455,7 +454,7 @@ static int hfsplus_symlink(struct inode *dir,
> struct dentry *dentry,
> if (res)
> goto out_err;
>
> - res = hfsplus_init_inode_security(inode, dir, &dentry-
> >d_name);
> + res = hfsplus_init_security(inode, dir, &dentry->d_name);
> if (res == -EOPNOTSUPP)
> res = 0; /* Operation is not supported. */
> else if (res) {
> @@ -496,7 +495,7 @@ static int hfsplus_mknod(struct inode *dir,
> struct dentry *dentry,
> if (res)
> goto failed_mknod;
>
> - res = hfsplus_init_inode_security(inode, dir, &dentry-
> >d_name);
> + res = hfsplus_init_security(inode, dir, &dentry->d_name);
> if (res == -EOPNOTSUPP)
> res = 0; /* Operation is not supported. */
> else if (res) {
> @@ -567,10 +566,6 @@ const struct inode_operations
> hfsplus_dir_inode_operations = {
> .mknod = hfsplus_mknod,
> .rename = hfsplus_rename,
> .listxattr = hfsplus_listxattr,
> -#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
> - .get_acl = hfsplus_get_posix_acl,
> - .set_acl = hfsplus_set_posix_acl,
> -#endif
> };
>
> const struct file_operations hfsplus_dir_operations = {
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index d9255abafb81..8e039435958a 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -31,7 +31,6 @@
> #define DBG_EXTENT 0x00000020
> #define DBG_BITMAP 0x00000040
> #define DBG_ATTR_MOD 0x00000080
> -#define DBG_ACL_MOD 0x00000100
>
> #if 0
> #define DBG_MASK (DBG_EXTENT|DBG_INODE|DBG_BNODE_MOD)
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index c824f702feec..8e9427a42b81 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -21,7 +21,6 @@
> #include "hfsplus_fs.h"
> #include "hfsplus_raw.h"
> #include "xattr.h"
> -#include "acl.h"
>
> static int hfsplus_readpage(struct file *file, struct page *page)
> {
> @@ -267,12 +266,6 @@ static int hfsplus_setattr(struct dentry
> *dentry, struct iattr *attr)
> setattr_copy(inode, attr);
> mark_inode_dirty(inode);
>
> - if (attr->ia_valid & ATTR_MODE) {
> - error = posix_acl_chmod(inode, inode->i_mode);
> - if (unlikely(error))
> - return error;
> - }
> -
> return 0;
> }
>
> @@ -336,10 +329,6 @@ int hfsplus_file_fsync(struct file *file, loff_t
> start, loff_t end,
> static const struct inode_operations hfsplus_file_inode_operations =
> {
> .setattr = hfsplus_setattr,
> .listxattr = hfsplus_listxattr,
> -#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
> - .get_acl = hfsplus_get_posix_acl,
> - .set_acl = hfsplus_set_posix_acl,
> -#endif
> };
>
> static const struct file_operations hfsplus_file_operations = {
> diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c
> deleted file mode 100644
> index 066114dcc3a2..000000000000
> --- a/fs/hfsplus/posix_acl.c
> +++ /dev/null
> @@ -1,144 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * linux/fs/hfsplus/posix_acl.c
> - *
> - * Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> - *
> - * Handler for Posix Access Control Lists (ACLs) support.
> - */
> -
> -#include "hfsplus_fs.h"
> -#include "xattr.h"
> -#include "acl.h"
> -
> -struct posix_acl *hfsplus_get_posix_acl(struct inode *inode, int
> type)
> -{
> - struct posix_acl *acl;
> - char *xattr_name;
> - char *value = NULL;
> - ssize_t size;
> -
> - hfs_dbg(ACL_MOD, "[%s]: ino %lu\n", __func__, inode->i_ino);
> -
> - switch (type) {
> - case ACL_TYPE_ACCESS:
> - xattr_name = XATTR_NAME_POSIX_ACL_ACCESS;
> - break;
> - case ACL_TYPE_DEFAULT:
> - xattr_name = XATTR_NAME_POSIX_ACL_DEFAULT;
> - break;
> - default:
> - return ERR_PTR(-EINVAL);
> - }
> -
> - size = __hfsplus_getxattr(inode, xattr_name, NULL, 0);
> -
> - if (size > 0) {
> - value = (char *)hfsplus_alloc_attr_entry();
> - if (unlikely(!value))
> - return ERR_PTR(-ENOMEM);
> - size = __hfsplus_getxattr(inode, xattr_name, value,
> size);
> - }
> -
> - if (size > 0)
> - acl = posix_acl_from_xattr(&init_user_ns, value,
> size);
> - else if (size == -ENODATA)
> - acl = NULL;
> - else
> - acl = ERR_PTR(size);
> -
> - hfsplus_destroy_attr_entry((hfsplus_attr_entry *)value);
> -
> - return acl;
> -}
> -
> -static int __hfsplus_set_posix_acl(struct inode *inode, struct
> posix_acl *acl,
> - int type)
> -{
> - int err;
> - char *xattr_name;
> - size_t size = 0;
> - char *value = NULL;
> -
> - hfs_dbg(ACL_MOD, "[%s]: ino %lu\n", __func__, inode->i_ino);
> -
> - switch (type) {
> - case ACL_TYPE_ACCESS:
> - xattr_name = XATTR_NAME_POSIX_ACL_ACCESS;
> - break;
> -
> - case ACL_TYPE_DEFAULT:
> - xattr_name = XATTR_NAME_POSIX_ACL_DEFAULT;
> - if (!S_ISDIR(inode->i_mode))
> - return acl ? -EACCES : 0;
> - break;
> -
> - default:
> - return -EINVAL;
> - }
> -
> - if (acl) {
> - size = posix_acl_xattr_size(acl->a_count);
> - if (unlikely(size > HFSPLUS_MAX_INLINE_DATA_SIZE))
> - return -ENOMEM;
> - value = (char *)hfsplus_alloc_attr_entry();
> - if (unlikely(!value))
> - return -ENOMEM;
> - err = posix_acl_to_xattr(&init_user_ns, acl, value,
> size);
> - if (unlikely(err < 0))
> - goto end_set_acl;
> - }
> -
> - err = __hfsplus_setxattr(inode, xattr_name, value, size, 0);
> -
> -end_set_acl:
> - hfsplus_destroy_attr_entry((hfsplus_attr_entry *)value);
> -
> - if (!err)
> - set_cached_acl(inode, type, acl);
> -
> - return err;
> -}
> -
> -int hfsplus_set_posix_acl(struct inode *inode, struct posix_acl
> *acl, int type)
> -{
> - int err;
> -
> - if (type == ACL_TYPE_ACCESS && acl) {
> - err = posix_acl_update_mode(inode, &inode->i_mode,
> &acl);
> - if (err)
> - return err;
> - }
> - return __hfsplus_set_posix_acl(inode, acl, type);
> -}
> -
> -int hfsplus_init_posix_acl(struct inode *inode, struct inode *dir)
> -{
> - int err = 0;
> - struct posix_acl *default_acl, *acl;
> -
> - hfs_dbg(ACL_MOD,
> - "[%s]: ino %lu, dir->ino %lu\n",
> - __func__, inode->i_ino, dir->i_ino);
> -
> - if (S_ISLNK(inode->i_mode))
> - return 0;
> -
> - err = posix_acl_create(dir, &inode->i_mode, &default_acl,
> &acl);
> - if (err)
> - return err;
> -
> - if (default_acl) {
> - err = __hfsplus_set_posix_acl(inode, default_acl,
> - ACL_TYPE_DEFAULT);
> - posix_acl_release(default_acl);
> - }
> -
> - if (acl) {
> - if (!err)
> - err = __hfsplus_set_posix_acl(inode, acl,
> - ACL_TYPE_ACCES
> S);
> - posix_acl_release(acl);
> - }
> - return err;
> -}
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index a6c0f54c48c3..0e88d1a270f0 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -562,8 +562,8 @@ static int hfsplus_fill_super(struct super_block
> *sb, void *data, int silent)
> goto out_put_hidden_dir;
> }
>
> - err = hfsplus_init_inode_security(sbi-
> >hidden_dir,
> - root
> , &str);
> + err = hfsplus_init_security(sbi->hidden_dir,
> + root, &str);
> if (err == -EOPNOTSUPP)
> err = 0; /* Operation is not
> supported. */
> else if (err) {
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index e538b758c448..d5403b4004c9 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -8,10 +8,8 @@
> */
>
> #include "hfsplus_fs.h"
> -#include <linux/posix_acl_xattr.h>
> #include <linux/nls.h>
> #include "xattr.h"
> -#include "acl.h"
>
> static int hfsplus_removexattr(struct inode *inode, const char
> *name);
>
> @@ -19,10 +17,6 @@ const struct xattr_handler
> *hfsplus_xattr_handlers[] = {
> &hfsplus_xattr_osx_handler,
> &hfsplus_xattr_user_handler,
> &hfsplus_xattr_trusted_handler,
> -#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
> - &posix_acl_access_xattr_handler,
> - &posix_acl_default_xattr_handler,
> -#endif
> &hfsplus_xattr_security_handler,
> NULL
> };
> diff --git a/fs/hfsplus/xattr.h b/fs/hfsplus/xattr.h
> index a4e611d69710..d14e362b3eba 100644
> --- a/fs/hfsplus/xattr.h
> +++ b/fs/hfsplus/xattr.h
> @@ -38,7 +38,4 @@ ssize_t hfsplus_listxattr(struct dentry *dentry,
> char *buffer, size_t size);
> int hfsplus_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr);
>
> -int hfsplus_init_inode_security(struct inode *inode, struct inode
> *dir,
> - const struct qstr *qstr);
> -
> #endif
> diff --git a/fs/hfsplus/xattr_security.c
> b/fs/hfsplus/xattr_security.c
> index f5550b006e88..cfbe6a3bfb1e 100644
> --- a/fs/hfsplus/xattr_security.c
> +++ b/fs/hfsplus/xattr_security.c
> @@ -12,7 +12,6 @@
>
> #include "hfsplus_fs.h"
> #include "xattr.h"
> -#include "acl.h"
>
> static int hfsplus_security_getxattr(const struct xattr_handler
> *handler,
> struct dentry *unused, struct
> inode *inode,
> @@ -72,18 +71,6 @@ int hfsplus_init_security(struct inode *inode,
> struct inode *dir,
> &hfsplus_initxattrs, NULL);
> }
>
> -int hfsplus_init_inode_security(struct inode *inode,
> - struct inode *dir,
> - const struct qstr
> *qstr)
> -{
> - int err;
> -
> - err = hfsplus_init_posix_acl(inode, dir);
> - if (!err)
> - err = hfsplus_init_security(inode, dir, qstr);
> - return err;
> -}
> -
> const struct xattr_handler hfsplus_xattr_security_handler = {
> .prefix = XATTR_SECURITY_PREFIX,
> .get = hfsplus_security_getxattr,
