| Message ID | 20240319161646.2153867-2-lizetao1@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ubifs: Support POSIX Access Control Lists (ACLs) | expand |
在 2024/3/20 0:16, Li Zetao 写道: > Implement the ACLs feature for ubifs based on vfs Posix ACLs, > details as follows: > * Initialize acl for newly created inode. > * Provides get/set interface to access ACLs. > > ACLs feature relies on xattr implementation which using specific key > names "system.posix_acl_default" and "system.posix_acl_access". Now Only > the v2 version of POSIX ACLs is supported, and ubifs does not need to > customize the storage format, which can simplify the implementation. > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > --- > fs/ubifs/acl.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ > fs/ubifs/ubifs.h | 13 +++++ > fs/ubifs/xattr.c | 1 - > 3 files changed, 153 insertions(+), 1 deletion(-) > create mode 100644 fs/ubifs/acl.c > > diff --git a/fs/ubifs/acl.c b/fs/ubifs/acl.c > new file mode 100644 > index 000000000000..253568baf097 > --- /dev/null > +++ b/fs/ubifs/acl.c > @@ -0,0 +1,140 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * This file is part of UBIFS. > + * > + * Copyright (C) 2024 Huawei Tech. Co., Ltd. > + * > + * Authors: Li Zetao <lizetao1@huawei.com> > + */ > + > +/* This file implements POSIX Access Control Lists (ACLs) */ > + > +#include "ubifs.h" > + > +#include <linux/posix_acl_xattr.h> > + > +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, bool rcu) > +{ > + char *xattr_value = NULL; > + const char *xattr_name; > + struct posix_acl *acl; > + ssize_t size; > + > + if (rcu) > + return ERR_PTR(-ECHILD); > + > + xattr_name = posix_acl_xattr_name(type); > + if (unlikely(!strcmp(xattr_name, ""))) > + return ERR_PTR(-EINVAL); The acl type has been guaranteed valid from vfs caller, there is no need to check converted name by 'strcmp', in theory, we can use it directly just like f2fs does. For this case, I suggest to unfold the posix_acl_xattr_name and convert it to corresponding name just like btrfs does. > + > + size = ubifs_xattr_get(inode, xattr_name, NULL, 0); > + if (size > 0) { > + xattr_value = kzalloc(size, GFP_KERNEL); > + if (unlikely(!xattr_value)) > + return ERR_PTR(-ENOMEM); > + > + size = ubifs_xattr_get(inode, xattr_name, xattr_value, size); > + } > + > + if (size > 0) > + acl = posix_acl_from_xattr(&init_user_ns, xattr_value, size); > + else if (size == -ENODATA || size == 0) > + acl = NULL; > + else > + acl = ERR_PTR(size); > + > + kfree(xattr_value); > + > + return acl; > +} > + > +static int __ubifs_set_acl(struct inode *inode, int type, struct posix_acl *acl, int flags) > +{ > + void *xattr_value = NULL; > + const char *xattr_name; > + size_t size = 0; > + int error; > + > + xattr_name = posix_acl_xattr_name(type); > + if (unlikely(!strcmp(xattr_name, ""))) > + return -EINVAL; > + > + if (unlikely(!strcmp(xattr_name, XATTR_NAME_POSIX_ACL_DEFAULT) && !S_ISDIR(inode->i_mode))) > + return acl ? -EACCES : 0; > + Similar to previous, replace above 6 lines, refer to __btrfs_set_acl but keep the error code same with __ext4_set_acl. > + if (acl) { > + size = posix_acl_xattr_size(acl->a_count); > + xattr_value = kmalloc(size, GFP_KERNEL); > + if (unlikely(!xattr_value)) > + return -ENOMEM; > + > + error = posix_acl_to_xattr(&init_user_ns, acl, xattr_value, size); > + if (unlikely(error < 0)) > + goto out; > + } > + > + error = ubifs_xattr_set(inode, xattr_name, xattr_value, size, flags, false); There are 2 situations here, Updating acl and Removing acl. For the later case, funcion vfs_remove_acl will remove corresponding xattr, the xattr removing function in ubifs is ubifs_xattr_remove. > + if (likely(!error)) I prefer to remove the 'likely', UBIFS limits the max xattr count for each file(Goto create_xattr), non zero error returned is a common case on a small LEB flash. > + set_cached_acl(inode, type, acl); > +out: > + kfree(xattr_value); > + return error; > +} > + > +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, struct posix_acl *acl, int type) > +{ > + struct inode *inode = d_inode(dentry); > + umode_t old_mode = inode->i_mode; > + int error; > + > + if (type == ACL_TYPE_ACCESS && acl) { > + error = posix_acl_update_mode(idmap, inode, &inode->i_mode, &acl); > + if (unlikely(error)) > + return error; > + } > + > + error = __ubifs_set_acl(inode, type, acl, 0); > + if (unlikely(error)) Mentioned in __ubifs_set_acl, error could be returned, just remove 'unlikely'. > + inode->i_mode = old_mode; > + > + return error; > + > +} > + > +/** > + * ubifs_init_acl - initialize the ACLs for a new inode. > + * @inode: newly created inode > + * @dir: parent directory inode > + * > + * This function initialize ACLs, including inheriting the initialize -> initializes > + * default ACLs of parent directory or modifying the default > + * ACLs according to the mode parameter in open() / creat() > + * system calls. > + */ > +int ubifs_init_acl(struct inode *inode, struct inode *dir) > +{ > + struct posix_acl *default_acl; > + struct posix_acl *acl; > + int error; > + > + error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl); > + if (unlikely(error)) > + return error; > + > + if (default_acl) { > + error = __ubifs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl, XATTR_CREATE); > + posix_acl_release(default_acl); > + } else { > + inode->i_default_acl = NULL; > + } > + > + if (acl) { > + if (likely(!error)) Remove 'likely'. > + error = __ubifs_set_acl(inode, ACL_TYPE_ACCESS, acl, XATTR_CREATE); > + posix_acl_release(acl); > + } else { > + inode->i_acl = NULL; > + } > + > + return error; > +} > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index 3916dc4f30ca..b0d3b076290d 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -2069,6 +2069,19 @@ static inline int ubifs_init_security(struct inode *dentry, > } > #endif > > +#ifdef CONFIG_UBIFS_FS_POSIX_ACL > +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, bool rcu); > +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, struct posix_acl *acl, int type); > +int ubifs_init_acl(struct inode *inode, struct inode *dir); > + > +#else /* CONFIG_UBIFS_FS_POSIX_ACL */ > +#define ubifs_get_inode_acl NULL > +#define ubifs_set_acl NULL > +static inline int ubifs_init_acl(struct inode *inode, struct inode *dir) > +{ > + return 0; > +} > +#endif /* CONFIG_UBIFS_FS_POSIX_ACL */ > > /* super.c */ > struct inode *ubifs_iget(struct super_block *sb, unsigned long inum); > diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c > index 0847db521984..eb1c1f5d10df 100644 > --- a/fs/ubifs/xattr.c > +++ b/fs/ubifs/xattr.c > @@ -40,7 +40,6 @@ > * in the VFS inode cache. The xentries are cached in the LNC cache (see > * tnc.c). > * > - * ACL support is not implemented. > */ > > #include "ubifs.h" >
Hi, On 2024/3/21 10:55, Zhihao Cheng wrote: > 在 2024/3/20 0:16, Li Zetao 写道: >> Implement the ACLs feature for ubifs based on vfs Posix ACLs, >> details as follows: >> * Initialize acl for newly created inode. >> * Provides get/set interface to access ACLs. >> >> ACLs feature relies on xattr implementation which using specific key >> names "system.posix_acl_default" and "system.posix_acl_access". Now Only >> the v2 version of POSIX ACLs is supported, and ubifs does not need to >> customize the storage format, which can simplify the implementation. >> >> Signed-off-by: Li Zetao <lizetao1@huawei.com> >> --- >> fs/ubifs/acl.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ >> fs/ubifs/ubifs.h | 13 +++++ >> fs/ubifs/xattr.c | 1 - >> 3 files changed, 153 insertions(+), 1 deletion(-) >> create mode 100644 fs/ubifs/acl.c >> >> diff --git a/fs/ubifs/acl.c b/fs/ubifs/acl.c >> new file mode 100644 >> index 000000000000..253568baf097 >> --- /dev/null >> +++ b/fs/ubifs/acl.c >> @@ -0,0 +1,140 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * This file is part of UBIFS. >> + * >> + * Copyright (C) 2024 Huawei Tech. Co., Ltd. >> + * >> + * Authors: Li Zetao <lizetao1@huawei.com> >> + */ >> + >> +/* This file implements POSIX Access Control Lists (ACLs) */ >> + >> +#include "ubifs.h" >> + >> +#include <linux/posix_acl_xattr.h> >> + >> +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, >> bool rcu) >> +{ >> + char *xattr_value = NULL; >> + const char *xattr_name; >> + struct posix_acl *acl; >> + ssize_t size; >> + >> + if (rcu) >> + return ERR_PTR(-ECHILD); >> + >> + xattr_name = posix_acl_xattr_name(type); >> + if (unlikely(!strcmp(xattr_name, ""))) >> + return ERR_PTR(-EINVAL); > The acl type has been guaranteed valid from vfs caller, there is no need > to check converted name by 'strcmp', in theory, we can use it directly > just like f2fs does. For this case, I suggest to unfold the > posix_acl_xattr_name and convert it to corresponding name just like > btrfs does. Ok, I will modify it in the next version. >> + >> + size = ubifs_xattr_get(inode, xattr_name, NULL, 0); >> + if (size > 0) { >> + xattr_value = kzalloc(size, GFP_KERNEL); >> + if (unlikely(!xattr_value)) >> + return ERR_PTR(-ENOMEM); >> + >> + size = ubifs_xattr_get(inode, xattr_name, xattr_value, size); >> + } >> + >> + if (size > 0) >> + acl = posix_acl_from_xattr(&init_user_ns, xattr_value, size); >> + else if (size == -ENODATA || size == 0) >> + acl = NULL; >> + else >> + acl = ERR_PTR(size); >> + >> + kfree(xattr_value); >> + >> + return acl; >> +} >> + >> +static int __ubifs_set_acl(struct inode *inode, int type, struct >> posix_acl *acl, int flags) >> +{ >> + void *xattr_value = NULL; >> + const char *xattr_name; >> + size_t size = 0; >> + int error; >> + > >> + xattr_name = posix_acl_xattr_name(type); >> + if (unlikely(!strcmp(xattr_name, ""))) >> + return -EINVAL; >> + >> + if (unlikely(!strcmp(xattr_name, XATTR_NAME_POSIX_ACL_DEFAULT) && >> !S_ISDIR(inode->i_mode))) >> + return acl ? -EACCES : 0; >> + > Similar to previous, replace above 6 lines, refer to __btrfs_set_acl but > keep the error code same with __ext4_set_acl. Ok. >> + if (acl) { >> + size = posix_acl_xattr_size(acl->a_count); >> + xattr_value = kmalloc(size, GFP_KERNEL); >> + if (unlikely(!xattr_value)) >> + return -ENOMEM; >> + >> + error = posix_acl_to_xattr(&init_user_ns, acl, xattr_value, >> size); >> + if (unlikely(error < 0)) >> + goto out; >> + } >> + >> + error = ubifs_xattr_set(inode, xattr_name, xattr_value, size, >> flags, false); > There are 2 situations here, Updating acl and Removing acl. For the > later case, funcion vfs_remove_acl will remove corresponding xattr, the > xattr removing function in ubifs is ubifs_xattr_remove. Thanks. Indeed, ubifs_xattr_set() is is different from other file systems, it will not delete xattr when value is NULL. >> + if (likely(!error)) > I prefer to remove the 'likely', UBIFS limits the max xattr count for > each file(Goto create_xattr), non zero error returned is a common case > on a small LEB flash. Ok. >> + set_cached_acl(inode, type, acl); >> +out: >> + kfree(xattr_value); >> + return error; >> +} >> + >> +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, >> struct posix_acl *acl, int type) >> +{ >> + struct inode *inode = d_inode(dentry); >> + umode_t old_mode = inode->i_mode; >> + int error; >> + >> + if (type == ACL_TYPE_ACCESS && acl) { >> + error = posix_acl_update_mode(idmap, inode, &inode->i_mode, >> &acl); >> + if (unlikely(error)) >> + return error; >> + } >> + >> + error = __ubifs_set_acl(inode, type, acl, 0); >> + if (unlikely(error)) > Mentioned in __ubifs_set_acl, error could be returned, just remove > 'unlikely'. Ok. >> + inode->i_mode = old_mode; >> + >> + return error; >> + >> +} >> + >> +/** >> + * ubifs_init_acl - initialize the ACLs for a new inode. >> + * @inode: newly created inode >> + * @dir: parent directory inode >> + * >> + * This function initialize ACLs, including inheriting the > initialize -> initializes Ok. >> + * default ACLs of parent directory or modifying the default >> + * ACLs according to the mode parameter in open() / creat() >> + * system calls. >> + */ >> +int ubifs_init_acl(struct inode *inode, struct inode *dir) >> +{ >> + struct posix_acl *default_acl; >> + struct posix_acl *acl; >> + int error; >> + >> + error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl); >> + if (unlikely(error)) >> + return error; >> + >> + if (default_acl) { >> + error = __ubifs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl, >> XATTR_CREATE); >> + posix_acl_release(default_acl); >> + } else { >> + inode->i_default_acl = NULL; >> + } >> + >> + if (acl) { >> + if (likely(!error)) > Remove 'likely'. Ok. >> + error = __ubifs_set_acl(inode, ACL_TYPE_ACCESS, acl, >> XATTR_CREATE); >> + posix_acl_release(acl); >> + } else { >> + inode->i_acl = NULL; >> + } >> + >> + return error; >> +} >> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h >> index 3916dc4f30ca..b0d3b076290d 100644 >> --- a/fs/ubifs/ubifs.h >> +++ b/fs/ubifs/ubifs.h >> @@ -2069,6 +2069,19 @@ static inline int ubifs_init_security(struct >> inode *dentry, >> } >> #endif >> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL >> +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, >> bool rcu); >> +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, >> struct posix_acl *acl, int type); >> +int ubifs_init_acl(struct inode *inode, struct inode *dir); >> + >> +#else /* CONFIG_UBIFS_FS_POSIX_ACL */ >> +#define ubifs_get_inode_acl NULL >> +#define ubifs_set_acl NULL >> +static inline int ubifs_init_acl(struct inode *inode, struct inode *dir) >> +{ >> + return 0; >> +} >> +#endif /* CONFIG_UBIFS_FS_POSIX_ACL */ >> /* super.c */ >> struct inode *ubifs_iget(struct super_block *sb, unsigned long inum); >> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c >> index 0847db521984..eb1c1f5d10df 100644 >> --- a/fs/ubifs/xattr.c >> +++ b/fs/ubifs/xattr.c >> @@ -40,7 +40,6 @@ >> * in the VFS inode cache. The xentries are cached in the LNC cache >> (see >> * tnc.c). >> * >> - * ACL support is not implemented. >> */ >> #include "ubifs.h" >> >
diff --git a/fs/ubifs/acl.c b/fs/ubifs/acl.c new file mode 100644 index 000000000000..253568baf097 --- /dev/null +++ b/fs/ubifs/acl.c @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * This file is part of UBIFS. + * + * Copyright (C) 2024 Huawei Tech. Co., Ltd. + * + * Authors: Li Zetao <lizetao1@huawei.com> + */ + +/* This file implements POSIX Access Control Lists (ACLs) */ + +#include "ubifs.h" + +#include <linux/posix_acl_xattr.h> + +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, bool rcu) +{ + char *xattr_value = NULL; + const char *xattr_name; + struct posix_acl *acl; + ssize_t size; + + if (rcu) + return ERR_PTR(-ECHILD); + + xattr_name = posix_acl_xattr_name(type); + if (unlikely(!strcmp(xattr_name, ""))) + return ERR_PTR(-EINVAL); + + size = ubifs_xattr_get(inode, xattr_name, NULL, 0); + if (size > 0) { + xattr_value = kzalloc(size, GFP_KERNEL); + if (unlikely(!xattr_value)) + return ERR_PTR(-ENOMEM); + + size = ubifs_xattr_get(inode, xattr_name, xattr_value, size); + } + + if (size > 0) + acl = posix_acl_from_xattr(&init_user_ns, xattr_value, size); + else if (size == -ENODATA || size == 0) + acl = NULL; + else + acl = ERR_PTR(size); + + kfree(xattr_value); + + return acl; +} + +static int __ubifs_set_acl(struct inode *inode, int type, struct posix_acl *acl, int flags) +{ + void *xattr_value = NULL; + const char *xattr_name; + size_t size = 0; + int error; + + xattr_name = posix_acl_xattr_name(type); + if (unlikely(!strcmp(xattr_name, ""))) + return -EINVAL; + + if (unlikely(!strcmp(xattr_name, XATTR_NAME_POSIX_ACL_DEFAULT) && !S_ISDIR(inode->i_mode))) + return acl ? -EACCES : 0; + + if (acl) { + size = posix_acl_xattr_size(acl->a_count); + xattr_value = kmalloc(size, GFP_KERNEL); + if (unlikely(!xattr_value)) + return -ENOMEM; + + error = posix_acl_to_xattr(&init_user_ns, acl, xattr_value, size); + if (unlikely(error < 0)) + goto out; + } + + error = ubifs_xattr_set(inode, xattr_name, xattr_value, size, flags, false); + if (likely(!error)) + set_cached_acl(inode, type, acl); +out: + kfree(xattr_value); + return error; +} + +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, struct posix_acl *acl, int type) +{ + struct inode *inode = d_inode(dentry); + umode_t old_mode = inode->i_mode; + int error; + + if (type == ACL_TYPE_ACCESS && acl) { + error = posix_acl_update_mode(idmap, inode, &inode->i_mode, &acl); + if (unlikely(error)) + return error; + } + + error = __ubifs_set_acl(inode, type, acl, 0); + if (unlikely(error)) + inode->i_mode = old_mode; + + return error; + +} + +/** + * ubifs_init_acl - initialize the ACLs for a new inode. + * @inode: newly created inode + * @dir: parent directory inode + * + * This function initialize ACLs, including inheriting the + * default ACLs of parent directory or modifying the default + * ACLs according to the mode parameter in open() / creat() + * system calls. + */ +int ubifs_init_acl(struct inode *inode, struct inode *dir) +{ + struct posix_acl *default_acl; + struct posix_acl *acl; + int error; + + error = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl); + if (unlikely(error)) + return error; + + if (default_acl) { + error = __ubifs_set_acl(inode, ACL_TYPE_DEFAULT, default_acl, XATTR_CREATE); + posix_acl_release(default_acl); + } else { + inode->i_default_acl = NULL; + } + + if (acl) { + if (likely(!error)) + error = __ubifs_set_acl(inode, ACL_TYPE_ACCESS, acl, XATTR_CREATE); + posix_acl_release(acl); + } else { + inode->i_acl = NULL; + } + + return error; +} diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 3916dc4f30ca..b0d3b076290d 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -2069,6 +2069,19 @@ static inline int ubifs_init_security(struct inode *dentry, } #endif +#ifdef CONFIG_UBIFS_FS_POSIX_ACL +struct posix_acl *ubifs_get_inode_acl(struct inode *inode, int type, bool rcu); +int ubifs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry, struct posix_acl *acl, int type); +int ubifs_init_acl(struct inode *inode, struct inode *dir); + +#else /* CONFIG_UBIFS_FS_POSIX_ACL */ +#define ubifs_get_inode_acl NULL +#define ubifs_set_acl NULL +static inline int ubifs_init_acl(struct inode *inode, struct inode *dir) +{ + return 0; +} +#endif /* CONFIG_UBIFS_FS_POSIX_ACL */ /* super.c */ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum); diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index 0847db521984..eb1c1f5d10df 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -40,7 +40,6 @@ * in the VFS inode cache. The xentries are cached in the LNC cache (see * tnc.c). * - * ACL support is not implemented. */ #include "ubifs.h"
Implement the ACLs feature for ubifs based on vfs Posix ACLs, details as follows: * Initialize acl for newly created inode. * Provides get/set interface to access ACLs. ACLs feature relies on xattr implementation which using specific key names "system.posix_acl_default" and "system.posix_acl_access". Now Only the v2 version of POSIX ACLs is supported, and ubifs does not need to customize the storage format, which can simplify the implementation. Signed-off-by: Li Zetao <lizetao1@huawei.com> --- fs/ubifs/acl.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ fs/ubifs/ubifs.h | 13 +++++ fs/ubifs/xattr.c | 1 - 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 fs/ubifs/acl.c