* [RFC 1/8] ecryptfs: Switch to generic xattr handlers
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 2/8] overlayfs: " Andreas Gruenbacher
` (8 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/ecryptfs/ecryptfs_kernel.h | 2 ++
fs/ecryptfs/inode.c | 48 +++++++++++++++++++++++++++++++++++--------
fs/ecryptfs/main.c | 1 +
3 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 6ff907f..706e1a4 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -716,4 +716,6 @@ int ecryptfs_set_f_namelen(long *namelen, long lower_namelen,
int ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat,
loff_t offset);
+extern const struct xattr_handler *ecryptfs_xattr_handlers[];
+
#endif /* #ifndef ECRYPTFS_KERNEL_H */
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 1ac631c..b6d9ce7 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1100,10 +1100,10 @@ const struct inode_operations ecryptfs_symlink_iops = {
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
.getattr = ecryptfs_getattr_link,
- .setxattr = ecryptfs_setxattr,
- .getxattr = ecryptfs_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = ecryptfs_removexattr
+ .removexattr = generic_removexattr
};
const struct inode_operations ecryptfs_dir_iops = {
@@ -1118,18 +1118,48 @@ const struct inode_operations ecryptfs_dir_iops = {
.rename = ecryptfs_rename,
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
- .setxattr = ecryptfs_setxattr,
- .getxattr = ecryptfs_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = ecryptfs_removexattr
+ .removexattr = generic_removexattr
};
const struct inode_operations ecryptfs_main_iops = {
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
.getattr = ecryptfs_getattr,
- .setxattr = ecryptfs_setxattr,
- .getxattr = ecryptfs_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = ecryptfs_removexattr
+ .removexattr = generic_removexattr
+};
+
+static int ecryptfs_xattr_get(const struct xattr_handler *handler,
+ struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
+{
+ return ecryptfs_getxattr(dentry, inode, name, buffer, size);
+}
+
+static int ecryptfs_xattr_set(const struct xattr_handler *handler,
+ struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
+{
+ if (value)
+ return ecryptfs_setxattr(dentry, name, value, size, flags);
+ else {
+ BUG_ON(flags != XATTR_REPLACE);
+ return ecryptfs_removexattr(dentry, name);
+ }
+}
+
+const struct xattr_handler ecryptfs_xattr_handler = {
+ .prefix = "", /* match anything */
+ .get = ecryptfs_xattr_get,
+ .set = ecryptfs_xattr_set,
+};
+
+const struct xattr_handler *ecryptfs_xattr_handlers[] = {
+ &ecryptfs_xattr_handler,
+ NULL
};
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 8b0b4a7..eb55f48 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -529,6 +529,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
/* ->kill_sb() will take care of sbi after that point */
sbi = NULL;
s->s_op = &ecryptfs_sops;
+ s->s_xattr = ecryptfs_xattr_handlers;
s->s_d_op = &ecryptfs_dops;
err = "Reading sb failed";
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 2/8] overlayfs: Switch to generic xattr handlers
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 1/8] ecryptfs: Switch to generic xattr handlers Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 3/8] fuse: " Andreas Gruenbacher
` (7 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/overlayfs/dir.c | 6 +++---
fs/overlayfs/inode.c | 52 ++++++++++++++++++++++++++++++++++++++----------
fs/overlayfs/overlayfs.h | 6 +-----
fs/overlayfs/super.c | 1 +
4 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index b3fc0a3..16ba0f8 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -967,8 +967,8 @@ const struct inode_operations ovl_dir_inode_operations = {
.mknod = ovl_mknod,
.permission = ovl_permission,
.getattr = ovl_dir_getattr,
- .setxattr = ovl_setxattr,
- .getxattr = ovl_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = ovl_removexattr,
+ .removexattr = generic_removexattr,
};
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index c7b31a0..d08c926 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -210,8 +210,8 @@ static bool ovl_is_private_xattr(const char *name)
return strncmp(name, OVL_XATTR_PRE_NAME, OVL_XATTR_PRE_LEN) == 0;
}
-int ovl_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags)
+static int ovl_setxattr(struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
{
int err;
struct dentry *upperdentry;
@@ -246,8 +246,8 @@ static bool ovl_need_xattr_filter(struct dentry *dentry,
return false;
}
-ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
- const char *name, void *value, size_t size)
+static ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, void *value, size_t size)
{
struct path realpath;
enum ovl_path_type type = ovl_path_real(dentry, &realpath);
@@ -290,7 +290,7 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
return res;
}
-int ovl_removexattr(struct dentry *dentry, const char *name)
+static int ovl_removexattr(struct dentry *dentry, const char *name)
{
int err;
struct path realpath;
@@ -374,10 +374,10 @@ static const struct inode_operations ovl_file_inode_operations = {
.setattr = ovl_setattr,
.permission = ovl_permission,
.getattr = ovl_getattr,
- .setxattr = ovl_setxattr,
- .getxattr = ovl_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = ovl_removexattr,
+ .removexattr = generic_removexattr,
};
static const struct inode_operations ovl_symlink_inode_operations = {
@@ -385,10 +385,10 @@ static const struct inode_operations ovl_symlink_inode_operations = {
.get_link = ovl_get_link,
.readlink = ovl_readlink,
.getattr = ovl_getattr,
- .setxattr = ovl_setxattr,
- .getxattr = ovl_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = ovl_removexattr,
+ .removexattr = generic_removexattr,
};
struct inode *ovl_new_inode(struct super_block *sb, umode_t mode,
@@ -433,3 +433,33 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode,
return inode;
}
+
+static int ovl_xattr_get(const struct xattr_handler *handler,
+ struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
+{
+ return ovl_getxattr(dentry, inode, name, buffer, size);
+}
+
+static int ovl_xattr_set(const struct xattr_handler *handler,
+ struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
+{
+ if (value)
+ return ovl_setxattr(dentry, name, value, size, flags);
+ else {
+ BUG_ON(flags != XATTR_REPLACE);
+ return ovl_removexattr(dentry, name);
+ }
+}
+
+const struct xattr_handler ovl_xattr_handler = {
+ .prefix = "", /* match anything */
+ .get = ovl_xattr_get,
+ .set = ovl_xattr_set,
+};
+
+const struct xattr_handler *ovl_xattr_handlers[] = {
+ &ovl_xattr_handler,
+ NULL
+};
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 99ec4b0..b7528e5 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -171,12 +171,7 @@ int ovl_check_d_type_supported(struct path *realpath);
/* inode.c */
int ovl_setattr(struct dentry *dentry, struct iattr *attr);
int ovl_permission(struct inode *inode, int mask);
-int ovl_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags);
-ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
- const char *name, void *value, size_t size);
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size);
-int ovl_removexattr(struct dentry *dentry, const char *name);
struct inode *ovl_d_select_inode(struct dentry *dentry, unsigned file_flags);
struct inode *ovl_new_inode(struct super_block *sb, umode_t mode,
@@ -186,6 +181,7 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to)
to->i_uid = from->i_uid;
to->i_gid = from->i_gid;
}
+extern const struct xattr_handler *ovl_xattr_handlers[];
/* dir.c */
extern const struct inode_operations ovl_dir_inode_operations;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 14cab38..46073fc 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1106,6 +1106,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
sb->s_magic = OVERLAYFS_SUPER_MAGIC;
sb->s_op = &ovl_super_operations;
+ sb->s_xattr = ovl_xattr_handlers;
sb->s_root = root_dentry;
sb->s_fs_info = ufs;
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 3/8] fuse: Switch to generic xattr handlers
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 1/8] ecryptfs: Switch to generic xattr handlers Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 2/8] overlayfs: " Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 4/8] evm: Turn evm_update_evmxattr into void function Andreas Gruenbacher
` (6 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/fuse/dir.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
fs/fuse/fuse_i.h | 2 ++
fs/fuse/inode.c | 1 +
3 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b618527..c36b3a3 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -13,6 +13,7 @@
#include <linux/sched.h>
#include <linux/namei.h>
#include <linux/slab.h>
+#include <linux/xattr.h>
static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
{
@@ -1883,10 +1884,10 @@ static const struct inode_operations fuse_dir_inode_operations = {
.mknod = fuse_mknod,
.permission = fuse_permission,
.getattr = fuse_getattr,
- .setxattr = fuse_setxattr,
- .getxattr = fuse_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = fuse_removexattr,
+ .removexattr = generic_removexattr,
};
static const struct file_operations fuse_dir_operations = {
@@ -1904,10 +1905,10 @@ static const struct inode_operations fuse_common_inode_operations = {
.setattr = fuse_setattr,
.permission = fuse_permission,
.getattr = fuse_getattr,
- .setxattr = fuse_setxattr,
- .getxattr = fuse_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = fuse_removexattr,
+ .removexattr = generic_removexattr,
};
static const struct inode_operations fuse_symlink_inode_operations = {
@@ -1915,10 +1916,10 @@ static const struct inode_operations fuse_symlink_inode_operations = {
.get_link = fuse_get_link,
.readlink = generic_readlink,
.getattr = fuse_getattr,
- .setxattr = fuse_setxattr,
- .getxattr = fuse_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = fuse_removexattr,
+ .removexattr = generic_removexattr,
};
void fuse_init_common(struct inode *inode)
@@ -1936,3 +1937,33 @@ void fuse_init_symlink(struct inode *inode)
{
inode->i_op = &fuse_symlink_inode_operations;
}
+
+static int fuse_xattr_get(const struct xattr_handler *handler,
+ struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
+{
+ return fuse_getxattr(dentry, inode, name, buffer, size);
+}
+
+static int fuse_xattr_set(const struct xattr_handler *handler,
+ struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
+{
+ if (value)
+ return fuse_setxattr(dentry, name, value, size, flags);
+ else {
+ BUG_ON(flags != XATTR_REPLACE);
+ return fuse_removexattr(dentry, name);
+ }
+}
+
+const struct xattr_handler fuse_xattr_handler = {
+ .prefix = "", /* match anything */
+ .get = fuse_xattr_get,
+ .set = fuse_xattr_set,
+};
+
+const struct xattr_handler *fuse_xattr_handlers[] = {
+ &fuse_xattr_handler,
+ NULL
+};
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index eddbe02..acf3481 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -956,4 +956,6 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
void fuse_set_initialized(struct fuse_conn *fc);
+extern const struct xattr_handler *fuse_xattr_handlers[];
+
#endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 4d69d5c..24d9669 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1058,6 +1058,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
}
sb->s_magic = FUSE_SUPER_MAGIC;
sb->s_op = &fuse_super_operations;
+ sb->s_xattr = fuse_xattr_handlers;
sb->s_maxbytes = MAX_LFS_FILESIZE;
sb->s_time_gran = 1;
sb->s_export_op = &fuse_export_operations;
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 4/8] evm: Turn evm_update_evmxattr into void function
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (2 preceding siblings ...)
2016-05-02 22:45 ` [RFC 3/8] fuse: " Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-04 7:23 ` James Morris
2016-05-02 22:45 ` [RFC 5/8] xattr: Add per-inode xattr handlers as a new inode operation Andreas Gruenbacher
` (5 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
The return value of evm_update_evmxattr is never used.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
security/integrity/evm/evm.h | 7 +++----
security/integrity/evm/evm_crypto.c | 14 ++++++--------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f5f1272..8b1cef07 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -39,10 +39,9 @@ extern struct crypto_shash *hash_tfm;
extern char *evm_config_xattrnames[];
int evm_init_key(void);
-int evm_update_evmxattr(struct dentry *dentry,
- const char *req_xattr_name,
- const char *req_xattr_value,
- size_t req_xattr_value_len);
+void evm_update_evmxattr(struct dentry *dentry, const char *req_xattr_name,
+ const char *req_xattr_value,
+ size_t req_xattr_value_len);
int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
const char *req_xattr_value,
size_t req_xattr_value_len, char *digest);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 30b6b7d0..3ac6407 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -239,24 +239,22 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
*
* Expects to be called with i_mutex locked.
*/
-int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
- const char *xattr_value, size_t xattr_value_len)
+void evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
+ const char *xattr_value, size_t xattr_value_len)
{
struct inode *inode = d_backing_inode(dentry);
struct evm_ima_xattr_data xattr_data;
- int rc = 0;
+ int rc;
rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
xattr_value_len, xattr_data.digest);
if (rc == 0) {
xattr_data.type = EVM_XATTR_HMAC;
- rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
- &xattr_data,
- sizeof(xattr_data), 0);
+ __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, &xattr_data,
+ sizeof(xattr_data), 0);
} else if (rc == -ENODATA && inode->i_op->removexattr) {
- rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
+ inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
}
- return rc;
}
int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 4/8] evm: Turn evm_update_evmxattr into void function
2016-05-02 22:45 ` [RFC 4/8] evm: Turn evm_update_evmxattr into void function Andreas Gruenbacher
@ 2016-05-04 7:23 ` James Morris
2016-05-04 11:20 ` Andreas Gruenbacher
0 siblings, 1 reply; 23+ messages in thread
From: James Morris @ 2016-05-04 7:23 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
On Tue, 3 May 2016, Andreas Gruenbacher wrote:
> The return value of evm_update_evmxattr is never used.
Perhaps it should be used, otherwise there are potential silent failure
paths in the EVM code.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 4/8] evm: Turn evm_update_evmxattr into void function
2016-05-04 7:23 ` James Morris
@ 2016-05-04 11:20 ` Andreas Gruenbacher
0 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-04 11:20 UTC (permalink / raw)
To: James Morris
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, LSM, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler
On Wed, May 4, 2016 at 9:23 AM, James Morris <jmorris@namei.org> wrote:
> On Tue, 3 May 2016, Andreas Gruenbacher wrote:
>
>> The return value of evm_update_evmxattr is never used.
>
> Perhaps it should be used, otherwise there are potential silent failure
> paths in the EVM code.
Yes, perhaps. That's not the only place in EVM / IMA where such
failures are silently
ignored though; the code seems to need some work.
Thanks,
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 5/8] xattr: Add per-inode xattr handlers as a new inode operation
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (3 preceding siblings ...)
2016-05-02 22:45 ` [RFC 4/8] evm: Turn evm_update_evmxattr into void function Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-14 18:21 ` Al Viro
2016-05-02 22:45 ` [RFC 6/8] xattr: Add __vfs_{get,set,remove}xattr helpers Andreas Gruenbacher
` (4 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
Per-inode xattr handlers allow to mark inodes as bad and dirs as "empty"
in the usual way even when using generic_getxattr, generic_setxattr, and
generic_removexattr. This brings us one step closer to getting rid of
the getxattr, setxattr, and removexattr inode operations.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/bad_inode.c | 36 +++++++++++++++++++++++-------------
fs/libfs.c | 29 +++++++++--------------------
fs/xattr.c | 17 ++++++++++-------
include/linux/fs.h | 1 +
include/linux/xattr.h | 6 ++++++
5 files changed, 49 insertions(+), 40 deletions(-)
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 72e35b7..e9227ea2d 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -14,6 +14,7 @@
#include <linux/time.h>
#include <linux/namei.h>
#include <linux/poll.h>
+#include <linux/xattr.h>
static int bad_file_open(struct inode *inode, struct file *filp)
{
@@ -100,28 +101,36 @@ static int bad_inode_setattr(struct dentry *direntry, struct iattr *attrs)
return -EIO;
}
-static int bad_inode_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags)
+static ssize_t bad_inode_listxattr(struct dentry *dentry, char *buffer,
+ size_t buffer_size)
{
return -EIO;
}
-static ssize_t bad_inode_getxattr(struct dentry *dentry, struct inode *inode,
- const char *name, void *buffer, size_t size)
+static int bad_inode_xattr_get(const struct xattr_handler *handler,
+ struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
{
return -EIO;
}
-static ssize_t bad_inode_listxattr(struct dentry *dentry, char *buffer,
- size_t buffer_size)
+static int bad_inode_xattr_set(const struct xattr_handler *handler,
+ struct dentry *dentry, const char *name,
+ const void *buffer, size_t size, int flags)
{
return -EIO;
}
-static int bad_inode_removexattr(struct dentry *dentry, const char *name)
-{
- return -EIO;
-}
+static const struct xattr_handler bad_inode_xattr_handler = {
+ .prefix = "", /* match anything */
+ .get = bad_inode_xattr_get,
+ .set = bad_inode_xattr_set,
+};
+
+static const struct xattr_handler *bad_inode_xattr_handlers[] = {
+ &bad_inode_xattr_handler,
+ NULL
+};
static const struct inode_operations bad_inode_ops =
{
@@ -142,10 +151,11 @@ static const struct inode_operations bad_inode_ops =
.permission = bad_inode_permission,
.getattr = bad_inode_getattr,
.setattr = bad_inode_setattr,
- .setxattr = bad_inode_setxattr,
- .getxattr = bad_inode_getxattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
.listxattr = bad_inode_listxattr,
- .removexattr = bad_inode_removexattr,
+ .removexattr = generic_removexattr,
+ .xattr = bad_inode_xattr_handlers,
};
diff --git a/fs/libfs.c b/fs/libfs.c
index 2a820bb..a7cea0a 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -15,6 +15,7 @@
#include <linux/exportfs.h>
#include <linux/writeback.h>
#include <linux/buffer_head.h> /* sync_mapping_buffers */
+#include <linux/xattr.h>
#include <asm/uaccess.h>
@@ -1122,37 +1123,25 @@ static int empty_dir_setattr(struct dentry *dentry, struct iattr *attr)
return -EPERM;
}
-static int empty_dir_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags)
-{
- return -EOPNOTSUPP;
-}
-
-static ssize_t empty_dir_getxattr(struct dentry *dentry, struct inode *inode,
- const char *name, void *value, size_t size)
-{
- return -EOPNOTSUPP;
-}
-
-static int empty_dir_removexattr(struct dentry *dentry, const char *name)
-{
- return -EOPNOTSUPP;
-}
-
static ssize_t empty_dir_listxattr(struct dentry *dentry, char *list, size_t size)
{
return -EOPNOTSUPP;
}
+static const struct xattr_handler *empty_dir_xattr_handlers[] = {
+ NULL
+};
+
static const struct inode_operations empty_dir_inode_operations = {
.lookup = empty_dir_lookup,
.permission = generic_permission,
.setattr = empty_dir_setattr,
.getattr = empty_dir_getattr,
- .setxattr = empty_dir_setxattr,
- .getxattr = empty_dir_getxattr,
- .removexattr = empty_dir_removexattr,
+ .setxattr = generic_setxattr,
+ .getxattr = generic_getxattr,
+ .removexattr = generic_removexattr,
.listxattr = empty_dir_listxattr,
+ .xattr = empty_dir_xattr_handlers,
};
static loff_t empty_dir_llseek(struct file *file, loff_t offset, int whence)
diff --git a/fs/xattr.c b/fs/xattr.c
index b11945e..68e093e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -649,7 +649,8 @@ strcmp_prefix(const char *a, const char *a_prefix)
* In order to implement different sets of xattr operations for each xattr
* prefix with the generic xattr API, a filesystem should create a
* null-terminated array of struct xattr_handler (one for each prefix) and
- * hang a pointer to it off of the s_xattr field of the superblock.
+ * hang a pointer to it off of the xattr field of the inode operations or
+ * the s_xattr field of the superblock.
*
* The generic_fooxattr() functions will use this list to dispatch xattr
* operations to the correct xattr_handler.
@@ -663,13 +664,14 @@ strcmp_prefix(const char *a, const char *a_prefix)
* Find the xattr_handler with the matching prefix.
*/
static const struct xattr_handler *
-xattr_resolve_name(const struct xattr_handler **handlers, const char **name)
+xattr_resolve_name(const struct dentry *dentry, const char **name)
{
- const struct xattr_handler *handler;
+ const struct xattr_handler *handler, **handlers;
if (!*name)
return NULL;
+ handlers = xattr_handlers(dentry->d_inode);
for_each_xattr_handler(handlers, handler) {
const char *n;
@@ -696,7 +698,7 @@ generic_getxattr(struct dentry *dentry, struct inode *inode,
{
const struct xattr_handler *handler;
- handler = xattr_resolve_name(dentry->d_sb->s_xattr, &name);
+ handler = xattr_resolve_name(dentry, &name);
if (IS_ERR(handler))
return PTR_ERR(handler);
return handler->get(handler, dentry, inode,
@@ -710,9 +712,10 @@ generic_getxattr(struct dentry *dentry, struct inode *inode,
ssize_t
generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
{
- const struct xattr_handler *handler, **handlers = dentry->d_sb->s_xattr;
+ const struct xattr_handler *handler, **handlers;
unsigned int size = 0;
+ handlers = xattr_handlers(dentry->d_inode);
if (!buffer) {
for_each_xattr_handler(handlers, handler) {
if (!handler->name ||
@@ -750,7 +753,7 @@ generic_setxattr(struct dentry *dentry, const char *name, const void *value, siz
if (size == 0)
value = ""; /* empty EA, do not remove */
- handler = xattr_resolve_name(dentry->d_sb->s_xattr, &name);
+ handler = xattr_resolve_name(dentry, &name);
if (IS_ERR(handler))
return PTR_ERR(handler);
return handler->set(handler, dentry, name, value, size, flags);
@@ -765,7 +768,7 @@ generic_removexattr(struct dentry *dentry, const char *name)
{
const struct xattr_handler *handler;
- handler = xattr_resolve_name(dentry->d_sb->s_xattr, &name);
+ handler = xattr_resolve_name(dentry, &name);
if (IS_ERR(handler))
return PTR_ERR(handler);
return handler->set(handler, dentry, name, NULL, 0, XATTR_REPLACE);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3489609..4f25cd3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1714,6 +1714,7 @@ struct inode_operations {
umode_t create_mode, int *opened);
int (*tmpfile) (struct inode *, struct dentry *, umode_t);
int (*set_acl)(struct inode *, struct posix_acl *, int);
+ const struct xattr_handler **xattr;
} ____cacheline_aligned;
ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 1cc4c57..2d30a3b 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -14,6 +14,7 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/spinlock.h>
+#include <linux/fs.h>
#include <uapi/linux/xattr.h>
struct inode;
@@ -64,6 +65,11 @@ static inline const char *xattr_prefix(const struct xattr_handler *handler)
return handler->prefix ?: handler->name;
}
+static inline const struct xattr_handler **xattr_handlers(struct inode *inode)
+{
+ return inode->i_op->xattr ?: inode->i_sb->s_xattr;
+}
+
struct simple_xattrs {
struct list_head head;
spinlock_t lock;
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 5/8] xattr: Add per-inode xattr handlers as a new inode operation
2016-05-02 22:45 ` [RFC 5/8] xattr: Add per-inode xattr handlers as a new inode operation Andreas Gruenbacher
@ 2016-05-14 18:21 ` Al Viro
0 siblings, 0 replies; 23+ messages in thread
From: Al Viro @ 2016-05-14 18:21 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: linux-fsdevel, Tyler Hicks, ecryptfs, Miklos Szeredi,
linux-unionfs, fuse-devel, Mimi Zohar, linux-ima-devel,
linux-security-module, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler
On Tue, May 03, 2016 at 12:45:15AM +0200, Andreas Gruenbacher wrote:
> Per-inode xattr handlers allow to mark inodes as bad and dirs as "empty"
> in the usual way even when using generic_getxattr, generic_setxattr, and
> generic_removexattr. This brings us one step closer to getting rid of
> the getxattr, setxattr, and removexattr inode operations.
This is an amazingly convoluted way of doing things. First of all, "empty"
case is not interesting - they might as well have used generic_...xattr for
the filesystem using them. And bad_inode... I'd rather have that checked
in generic_getxattr() et.al. I mean, explicit
if (unlikely(is_bad_inode(inode)))
return -EIO;
... go using ->i_sb->s_xattr
in there won't cost more than your variant and it avoids having a flag
misguised as a pointer to secondary method table in every inode_operations.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 6/8] xattr: Add __vfs_{get,set,remove}xattr helpers
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (4 preceding siblings ...)
2016-05-02 22:45 ` [RFC 5/8] xattr: Add per-inode xattr handlers as a new inode operation Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 7/8] xattr: Stop calling {get,set,remove}xattr inode operations Andreas Gruenbacher
` (3 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
Right now, various places in the kernel check for the existence of
getxattr, setxattr, and removexattr inode operations and directly call
those operations. Switch to helper functions for that instead.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/cachefiles/bind.c | 4 ++--
fs/cachefiles/namei.c | 4 ++--
fs/ecryptfs/inode.c | 25 ++++---------------
fs/ecryptfs/mmap.c | 15 ++++--------
fs/overlayfs/copy_up.c | 4 ----
fs/overlayfs/super.c | 4 ++--
fs/xattr.c | 45 +++++++++++++++++++++++++++--------
include/linux/xattr.h | 3 +++
security/commoncap.c | 25 ++++++++-----------
security/integrity/evm/evm_crypto.c | 8 ++-----
security/integrity/evm/evm_main.c | 5 +---
security/integrity/ima/ima_appraise.c | 23 +++++++-----------
security/selinux/hooks.c | 28 +++++-----------------
security/smack/smack_lsm.c | 28 +++++++++++-----------
14 files changed, 94 insertions(+), 127 deletions(-)
diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
index 6af790f..f57bd4a 100644
--- a/fs/cachefiles/bind.c
+++ b/fs/cachefiles/bind.c
@@ -20,6 +20,7 @@
#include <linux/mount.h>
#include <linux/statfs.h>
#include <linux/ctype.h>
+#include <linux/xattr.h>
#include "internal.h"
static int cachefiles_daemon_add_cache(struct cachefiles_cache *caches);
@@ -126,8 +127,7 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache)
if (d_is_negative(root) ||
!d_backing_inode(root)->i_op->lookup ||
!d_backing_inode(root)->i_op->mkdir ||
- !d_backing_inode(root)->i_op->setxattr ||
- !d_backing_inode(root)->i_op->getxattr ||
+ !xattr_handlers(d_backing_inode(root)) ||
!root->d_sb->s_op->statfs ||
!root->d_sb->s_op->sync_fs)
goto error_unsupported;
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 4ae7500..d0dfa66 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -20,6 +20,7 @@
#include <linux/namei.h>
#include <linux/security.h>
#include <linux/slab.h>
+#include <linux/xattr.h>
#include "internal.h"
#define CACHEFILES_KEYBUF_SIZE 512
@@ -798,8 +799,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
}
ret = -EPERM;
- if (!d_backing_inode(subdir)->i_op->setxattr ||
- !d_backing_inode(subdir)->i_op->getxattr ||
+ if (!xattr_handlers(d_backing_inode(subdir)) ||
!d_backing_inode(subdir)->i_op->lookup ||
!d_backing_inode(subdir)->i_op->mkdir ||
!d_backing_inode(subdir)->i_op->create ||
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index b6d9ce7..f8729cb 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1020,15 +1020,9 @@ ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
struct dentry *lower_dentry;
lower_dentry = ecryptfs_dentry_to_lower(dentry);
- if (!d_inode(lower_dentry)->i_op->setxattr) {
- rc = -EOPNOTSUPP;
- goto out;
- }
-
rc = vfs_setxattr(lower_dentry, name, value, size, flags);
if (!rc && d_really_is_positive(dentry))
fsstack_copy_attr_all(d_inode(dentry), d_inode(lower_dentry));
-out:
return rc;
}
@@ -1036,17 +1030,11 @@ ssize_t
ecryptfs_getxattr_lower(struct dentry *lower_dentry, struct inode *lower_inode,
const char *name, void *value, size_t size)
{
- int rc = 0;
+ int rc;
- if (!lower_inode->i_op->getxattr) {
- rc = -EOPNOTSUPP;
- goto out;
- }
inode_lock(lower_inode);
- rc = lower_inode->i_op->getxattr(lower_dentry, lower_inode,
- name, value, size);
+ rc = __vfs_getxattr(lower_dentry, lower_inode, name, value, size);
inode_unlock(lower_inode);
-out:
return rc;
}
@@ -1079,18 +1067,13 @@ out:
static int ecryptfs_removexattr(struct dentry *dentry, const char *name)
{
- int rc = 0;
+ int rc;
struct dentry *lower_dentry;
lower_dentry = ecryptfs_dentry_to_lower(dentry);
- if (!d_inode(lower_dentry)->i_op->removexattr) {
- rc = -EOPNOTSUPP;
- goto out;
- }
inode_lock(d_inode(lower_dentry));
- rc = d_inode(lower_dentry)->i_op->removexattr(lower_dentry, name);
+ rc = __vfs_removexattr(lower_dentry, name);
inode_unlock(d_inode(lower_dentry));
-out:
return rc;
}
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 39e4381..3e9fc6d 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -32,6 +32,7 @@
#include <linux/file.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
+#include <linux/xattr.h>
#include <asm/unaligned.h>
#include "ecryptfs_kernel.h"
@@ -422,12 +423,6 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *ecryptfs_inode)
struct inode *lower_inode = d_inode(lower_dentry);
int rc;
- if (!lower_inode->i_op->getxattr || !lower_inode->i_op->setxattr) {
- printk(KERN_WARNING
- "No support for setting xattr in lower filesystem\n");
- rc = -ENOSYS;
- goto out;
- }
xattr_virt = kmem_cache_alloc(ecryptfs_xattr_cache, GFP_KERNEL);
if (!xattr_virt) {
printk(KERN_ERR "Out of memory whilst attempting to write "
@@ -436,14 +431,12 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *ecryptfs_inode)
goto out;
}
inode_lock(lower_inode);
- size = lower_inode->i_op->getxattr(lower_dentry, lower_inode,
- ECRYPTFS_XATTR_NAME,
- xattr_virt, PAGE_CACHE_SIZE);
+ size = __vfs_getxattr(lower_dentry, lower_inode, ECRYPTFS_XATTR_NAME,
+ xattr_virt, PAGE_CACHE_SIZE);
if (size < 0)
size = 8;
put_unaligned_be64(i_size_read(ecryptfs_inode), xattr_virt);
- rc = lower_inode->i_op->setxattr(lower_dentry, ECRYPTFS_XATTR_NAME,
- xattr_virt, size, 0);
+ rc = __vfs_setxattr(lower_dentry, ECRYPTFS_XATTR_NAME, xattr_virt, size, 0);
inode_unlock(lower_inode);
if (rc)
printk(KERN_ERR "Error whilst attempting to write inode size "
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index cc514da..3a2a808 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -58,10 +58,6 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
char *buf, *name, *value = NULL;
int uninitialized_var(error);
- if (!old->d_inode->i_op->getxattr ||
- !new->d_inode->i_op->getxattr)
- return 0;
-
list_size = vfs_listxattr(old, NULL, 0);
if (list_size <= 0) {
if (list_size == -EOPNOTSUPP)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 46073fc..dfa45b5 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -271,10 +271,10 @@ static bool ovl_is_opaquedir(struct dentry *dentry)
char val;
struct inode *inode = dentry->d_inode;
- if (!S_ISDIR(inode->i_mode) || !inode->i_op->getxattr)
+ if (!S_ISDIR(inode->i_mode))
return false;
- res = inode->i_op->getxattr(dentry, inode, OVL_XATTR_OPAQUE, &val, 1);
+ res = __vfs_getxattr(dentry, inode, OVL_XATTR_OPAQUE, &val, 1);
if (res == 1 && val == 'y')
return true;
diff --git a/fs/xattr.c b/fs/xattr.c
index 68e093e..f899196 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -73,6 +73,18 @@ xattr_permission(struct inode *inode, const char *name, int mask)
return inode_permission(inode, mask);
}
+int
+__vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+ size_t size, int flags)
+{
+ struct inode *inode = dentry->d_inode;
+
+ if (!inode->i_op->setxattr)
+ return -EOPNOTSUPP;
+ return inode->i_op->setxattr(dentry, name, value, size, flags);
+}
+EXPORT_SYMBOL(__vfs_setxattr);
+
/**
* __vfs_setxattr_noperm - perform setxattr operation without performing
* permission checks.
@@ -100,7 +112,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
if (issec)
inode->i_flags &= ~S_NOSEC;
if (inode->i_op->setxattr) {
- error = inode->i_op->setxattr(dentry, name, value, size, flags);
+ error = __vfs_setxattr(dentry, name, value, size, flags);
if (!error) {
fsnotify_xattr(dentry);
security_inode_post_setxattr(dentry, name, value,
@@ -209,6 +221,16 @@ vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
}
ssize_t
+__vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,
+ void *value, size_t size)
+{
+ if (!inode->i_op->getxattr)
+ return -EOPNOTSUPP;
+ return inode->i_op->getxattr(dentry, inode, name, value, size);
+}
+EXPORT_SYMBOL(__vfs_getxattr);
+
+ssize_t
vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size)
{
struct inode *inode = dentry->d_inode;
@@ -235,12 +257,7 @@ vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size)
return ret;
}
nolsm:
- if (inode->i_op->getxattr)
- error = inode->i_op->getxattr(dentry, inode, name, value, size);
- else
- error = -EOPNOTSUPP;
-
- return error;
+ return __vfs_getxattr(dentry, inode, name, value, size);
}
EXPORT_SYMBOL_GPL(vfs_getxattr);
@@ -265,13 +282,21 @@ vfs_listxattr(struct dentry *d, char *list, size_t size)
EXPORT_SYMBOL_GPL(vfs_listxattr);
int
-vfs_removexattr(struct dentry *dentry, const char *name)
+__vfs_removexattr(struct dentry *dentry, const char *name)
{
struct inode *inode = dentry->d_inode;
- int error;
if (!inode->i_op->removexattr)
return -EOPNOTSUPP;
+ return inode->i_op->removexattr(dentry, name);
+}
+EXPORT_SYMBOL(__vfs_removexattr);
+
+int
+vfs_removexattr(struct dentry *dentry, const char *name)
+{
+ struct inode *inode = dentry->d_inode;
+ int error;
error = xattr_permission(inode, name, MAY_WRITE);
if (error)
@@ -282,7 +307,7 @@ vfs_removexattr(struct dentry *dentry, const char *name)
if (error)
goto out;
- error = inode->i_op->removexattr(dentry, name);
+ error = __vfs_removexattr(dentry, name);
if (!error) {
fsnotify_xattr(dentry);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 2d30a3b..76557ce 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -47,10 +47,13 @@ struct xattr {
};
ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
+ssize_t __vfs_getxattr(struct dentry *, struct inode *, const char *, void *, size_t);
ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
+int __vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int);
int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
+int __vfs_removexattr(struct dentry *, const char *);
int vfs_removexattr(struct dentry *, const char *);
ssize_t generic_getxattr(struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size);
diff --git a/security/commoncap.c b/security/commoncap.c
index a042077..b23de30 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -310,13 +310,8 @@ int cap_inode_need_killpriv(struct dentry *dentry)
struct inode *inode = d_backing_inode(dentry);
int error;
- if (!inode->i_op->getxattr)
- return 0;
-
- error = inode->i_op->getxattr(dentry, inode, XATTR_NAME_CAPS, NULL, 0);
- if (error <= 0)
- return 0;
- return 1;
+ error = __vfs_getxattr(dentry, inode, XATTR_NAME_CAPS, NULL, 0);
+ return error > 0;
}
/**
@@ -329,12 +324,12 @@ int cap_inode_need_killpriv(struct dentry *dentry)
*/
int cap_inode_killpriv(struct dentry *dentry)
{
- struct inode *inode = d_backing_inode(dentry);
-
- if (!inode->i_op->removexattr)
- return 0;
+ int error;
- return inode->i_op->removexattr(dentry, XATTR_NAME_CAPS);
+ error = __vfs_removexattr(dentry, XATTR_NAME_CAPS);
+ if (error == -EOPNOTSUPP)
+ error = 0;
+ return error;
}
/*
@@ -394,11 +389,11 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data
memset(cpu_caps, 0, sizeof(struct cpu_vfs_cap_data));
- if (!inode || !inode->i_op->getxattr)
+ if (!inode)
return -ENODATA;
- size = inode->i_op->getxattr((struct dentry *)dentry, inode,
- XATTR_NAME_CAPS, &caps, XATTR_CAPS_SZ);
+ size = __vfs_getxattr((struct dentry *)dentry, inode,
+ XATTR_NAME_CAPS, &caps, XATTR_CAPS_SZ);
if (size == -ENODATA || size == -EOPNOTSUPP)
/* no data, that's ok */
return -ENODATA;
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 3ac6407..22bb69d 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -182,8 +182,6 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
int error;
int size;
- if (!inode->i_op->getxattr)
- return -EOPNOTSUPP;
desc = init_desc(type);
if (IS_ERR(desc))
return PTR_ERR(desc);
@@ -242,7 +240,6 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
void evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
const char *xattr_value, size_t xattr_value_len)
{
- struct inode *inode = d_backing_inode(dentry);
struct evm_ima_xattr_data xattr_data;
int rc;
@@ -252,9 +249,8 @@ void evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
xattr_data.type = EVM_XATTR_HMAC;
__vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, &xattr_data,
sizeof(xattr_data), 0);
- } else if (rc == -ENODATA && inode->i_op->removexattr) {
- inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
- }
+ } else if (rc == -ENODATA)
+ __vfs_removexattr(dentry, XATTR_NAME_EVM);
}
int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b9e2628..8e9dbad 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -78,11 +78,8 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
int error;
int count = 0;
- if (!inode->i_op->getxattr)
- return -EOPNOTSUPP;
-
for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
- error = inode->i_op->getxattr(dentry, inode, *xattr, NULL, 0);
+ error = __vfs_getxattr(dentry, inode, *xattr, NULL, 0);
if (error < 0) {
if (error == -ENODATA)
continue;
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 6b4694a..970a8c1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -165,13 +165,13 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value)
{
- struct inode *inode = d_backing_inode(dentry);
-
- if (!inode->i_op->getxattr)
- return 0;
+ ssize_t ret;
- return vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
- 0, GFP_NOFS);
+ ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
+ 0, GFP_NOFS);
+ if (ret == -EOPNOTSUPP)
+ ret = 0;
+ return ret;
}
/*
@@ -195,9 +195,6 @@ int ima_appraise_measurement(enum ima_hooks func,
enum integrity_status status = INTEGRITY_UNKNOWN;
int rc = xattr_len, hash_start = 0;
- if (!inode->i_op->getxattr)
- return INTEGRITY_UNKNOWN;
-
if (rc <= 0) {
if (rc && rc != -ENODATA)
goto out;
@@ -317,10 +314,9 @@ void ima_inode_post_setattr(struct dentry *dentry)
{
struct inode *inode = d_backing_inode(dentry);
struct integrity_iint_cache *iint;
- int must_appraise, rc;
+ int must_appraise;
- if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)
- || !inode->i_op->removexattr)
+ if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode))
return;
must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR);
@@ -333,8 +329,7 @@ void ima_inode_post_setattr(struct dentry *dentry)
iint->flags |= IMA_APPRAISE;
}
if (!must_appraise)
- rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA);
- return;
+ __vfs_removexattr(dentry, XATTR_NAME_IMA);
}
/*
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 469f5c7..73e7f9c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -500,14 +500,7 @@ static int sb_finish_set_opts(struct super_block *sb)
the root directory. -ENODATA is ok, as this may be
the first boot of the SELinux kernel before we have
assigned xattr values to the filesystem. */
- if (!root_inode->i_op->getxattr) {
- printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
- "xattr support\n", sb->s_id, sb->s_type->name);
- rc = -EOPNOTSUPP;
- goto out;
- }
- rc = root_inode->i_op->getxattr(root, root_inode,
- XATTR_NAME_SELINUX, NULL, 0);
+ rc = __vfs_getxattr(root, root_inode, XATTR_NAME_SELINUX, NULL, 0);
if (rc < 0 && rc != -ENODATA) {
if (rc == -EOPNOTSUPP)
printk(KERN_WARNING "SELinux: (dev %s, type "
@@ -1378,11 +1371,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
case SECURITY_FS_USE_NATIVE:
break;
case SECURITY_FS_USE_XATTR:
- if (!inode->i_op->getxattr) {
- isec->sid = sbsec->def_sid;
- break;
- }
-
/* Need a dentry, since the xattr API requires one.
Life would be simpler if we could just pass the inode. */
if (opt_dentry) {
@@ -1413,14 +1401,12 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
goto out_unlock;
}
context[len] = '\0';
- rc = inode->i_op->getxattr(dentry, inode, XATTR_NAME_SELINUX,
- context, len);
+ rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len);
if (rc == -ERANGE) {
kfree(context);
/* Need a larger buffer. Query for the right size. */
- rc = inode->i_op->getxattr(dentry, inode, XATTR_NAME_SELINUX,
- NULL, 0);
+ rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0);
if (rc < 0) {
dput(dentry);
goto out_unlock;
@@ -1433,20 +1419,18 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
goto out_unlock;
}
context[len] = '\0';
- rc = inode->i_op->getxattr(dentry, inode,
- XATTR_NAME_SELINUX,
- context, len);
+ rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len);
}
dput(dentry);
if (rc < 0) {
- if (rc != -ENODATA) {
+ if (rc != -EOPNOTSUPP && rc != -ENODATA) {
printk(KERN_WARNING "SELinux: %s: getxattr returned "
"%d for dev=%s ino=%ld\n", __func__,
-rc, inode->i_sb->s_id, inode->i_ino);
kfree(context);
goto out_unlock;
}
- /* Map ENODATA to the default file SID */
+ /* Map EOPNOTSUPP and ENODATA to the default file SID */
sid = sbsec->def_sid;
rc = 0;
} else {
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ff2b8c3..e511438 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -265,14 +265,11 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip,
char *buffer;
struct smack_known *skp = NULL;
- if (ip->i_op->getxattr == NULL)
- return ERR_PTR(-EOPNOTSUPP);
-
buffer = kzalloc(SMK_LONGLABEL, GFP_KERNEL);
if (buffer == NULL)
return ERR_PTR(-ENOMEM);
- rc = ip->i_op->getxattr(dp, ip, name, buffer, SMK_LONGLABEL);
+ rc = __vfs_getxattr(dp, ip, name, buffer, SMK_LONGLABEL);
if (rc < 0)
skp = ERR_PTR(rc);
else if (rc == 0)
@@ -3484,14 +3481,6 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
break;
}
/*
- * No xattr support means, alas, no SMACK label.
- * Use the aforeapplied default.
- * It would be curious if the label of the task
- * does not match that assigned.
- */
- if (inode->i_op->getxattr == NULL)
- break;
- /*
* Get the dentry for xattr.
*/
dp = dget(opt_dentry);
@@ -3514,14 +3503,25 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
*/
if (isp->smk_flags & SMK_INODE_CHANGED) {
isp->smk_flags &= ~SMK_INODE_CHANGED;
- rc = inode->i_op->setxattr(dp,
+ rc = __vfs_setxattr(dp,
XATTR_NAME_SMACKTRANSMUTE,
TRANS_TRUE, TRANS_TRUE_SIZE,
0);
} else {
- rc = inode->i_op->getxattr(dp, inode,
+ rc = __vfs_getxattr(dp, inode,
XATTR_NAME_SMACKTRANSMUTE, trattr,
TRANS_TRUE_SIZE);
+ if (rc == -EOPNOTSUPP) {
+ /*
+ * No xattr support means, alas, no
+ * SMACK label. Use the aforeapplied
+ * default. It would be curious if the
+ * label of the task does not match
+ * that assigned.
+ */
+ dput(dp);
+ break;
+ }
if (rc >= 0 && strncmp(trattr, TRANS_TRUE,
TRANS_TRUE_SIZE) != 0)
rc = -EINVAL;
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 7/8] xattr: Stop calling {get,set,remove}xattr inode operations
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (5 preceding siblings ...)
2016-05-02 22:45 ` [RFC 6/8] xattr: Add __vfs_{get,set,remove}xattr helpers Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-02 22:45 ` [RFC 8/8] xattr: Remove generic xattr handlers Andreas Gruenbacher
` (2 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
All filesystems that support xattrs now do so via xattr handlers. They
all define sb->s_xattr or iop->xattr, and their getxattr, setxattr, and
removexattr inode operations use the generic methods.
On filesystems that don't support xattrs, the getxattr, setxattr, and
removexattr inode operations are still NULL, and sb->s_xattr and
iop->xattr are also NULL.
This means that we can now remove the getxattr, setxattr, and
removexattr inode operations and directly call the generic handlers, or
better, inline expand those handlers into fs/xattr.c.
Instead of checking if the {get,set,remove}xattr inode operation is
defined in __vfs_{get,set,removexattr}, respectively, use
xattr_resolve_name to find the matching handler and then check if the
requested operation (->get or ->set) is defined in that handler. We
need to check for NULL handlers in xattr_resolve_name now; so far,
xattr_resolve_name could just assume that sb->s_xattr is defined.
Unlike with the generic_{get,set,remove}xattr inode operations, it's now
possible to define get and set operations in some xattr handlers but not
in others.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xattr.c | 151 +++++++++++++++++++++++++++++++++----------------------------
1 file changed, 83 insertions(+), 68 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index f899196..a3fa123 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -24,6 +24,60 @@
#include <asm/uaccess.h>
+static const char *
+strcmp_prefix(const char *a, const char *a_prefix)
+{
+ while (*a_prefix && *a == *a_prefix) {
+ a++;
+ a_prefix++;
+ }
+ return *a_prefix ? NULL : a;
+}
+
+/*
+ * In order to implement different sets of xattr operations for each xattr
+ * prefix with the generic xattr API, a filesystem should create a
+ * null-terminated array of struct xattr_handler (one for each prefix) and
+ * hang a pointer to it off of the xattr field of the inode operations or
+ * the s_xattr field of the superblock.
+ *
+ * The generic_fooxattr() functions will use this list to dispatch xattr
+ * operations to the correct xattr_handler.
+ */
+#define for_each_xattr_handler(handlers, handler) \
+ for ((handler) = *(handlers)++; \
+ (handler) != NULL; \
+ (handler) = *(handlers)++)
+
+/*
+ * Find the xattr_handler with the matching prefix.
+ */
+static const struct xattr_handler *
+xattr_resolve_name(const struct dentry *dentry, const char **name)
+{
+ const struct xattr_handler *handler, **handlers;
+
+ if (!*name)
+ return NULL;
+
+ handlers = xattr_handlers(dentry->d_inode);
+ for_each_xattr_handler(handlers, handler) {
+ const char *n;
+
+ n = strcmp_prefix(*name, xattr_prefix(handler));
+ if (n) {
+ if (!handler->prefix ^ !*n) {
+ if (*n)
+ continue;
+ return ERR_PTR(-EINVAL);
+ }
+ *name = n;
+ return handler;
+ }
+ }
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
/*
* Check permissions for extended attribute access. This is a bit complicated
* because different namespaces have very different rules.
@@ -77,11 +131,16 @@ int
__vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags)
{
- struct inode *inode = dentry->d_inode;
+ const struct xattr_handler *handler;
- if (!inode->i_op->setxattr)
+ handler = xattr_resolve_name(dentry, &name);
+ if (IS_ERR(handler))
+ return PTR_ERR(handler);
+ if (!handler->set)
return -EOPNOTSUPP;
- return inode->i_op->setxattr(dentry, name, value, size, flags);
+ if (size == 0)
+ value = ""; /* empty EA, do not remove */
+ return handler->set(handler, dentry, name, value, size, flags);
}
EXPORT_SYMBOL(__vfs_setxattr);
@@ -111,7 +170,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
if (issec)
inode->i_flags &= ~S_NOSEC;
- if (inode->i_op->setxattr) {
+ if (xattr_handlers(inode)) {
error = __vfs_setxattr(dentry, name, value, size, flags);
if (!error) {
fsnotify_xattr(dentry);
@@ -193,6 +252,7 @@ ssize_t
vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
size_t xattr_size, gfp_t flags)
{
+ const struct xattr_handler *handler;
struct inode *inode = dentry->d_inode;
char *value = *xattr_value;
int error;
@@ -201,10 +261,12 @@ vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
if (error)
return error;
- if (!inode->i_op->getxattr)
+ handler = xattr_resolve_name(dentry, &name);
+ if (IS_ERR(handler))
+ return PTR_ERR(handler);
+ if (!handler->get)
return -EOPNOTSUPP;
-
- error = inode->i_op->getxattr(dentry, inode, name, NULL, 0);
+ error = handler->get(handler, dentry, inode, name, NULL, 0);
if (error < 0)
return error;
@@ -215,7 +277,7 @@ vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value,
memset(value, 0, error + 1);
}
- error = inode->i_op->getxattr(dentry, inode, name, value, error);
+ error = handler->get(handler, dentry, inode, name, value, error);
*xattr_value = value;
return error;
}
@@ -224,9 +286,14 @@ ssize_t
__vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,
void *value, size_t size)
{
- if (!inode->i_op->getxattr)
+ const struct xattr_handler *handler;
+
+ handler = xattr_resolve_name(dentry, &name);
+ if (IS_ERR(handler))
+ return PTR_ERR(handler);
+ if (!handler->get)
return -EOPNOTSUPP;
- return inode->i_op->getxattr(dentry, inode, name, value, size);
+ return handler->get(handler, dentry, inode, name, value, size);
}
EXPORT_SYMBOL(__vfs_getxattr);
@@ -284,11 +351,14 @@ EXPORT_SYMBOL_GPL(vfs_listxattr);
int
__vfs_removexattr(struct dentry *dentry, const char *name)
{
- struct inode *inode = dentry->d_inode;
+ const struct xattr_handler *handler;
- if (!inode->i_op->removexattr)
+ handler = xattr_resolve_name(dentry, &name);
+ if (IS_ERR(handler))
+ return PTR_ERR(handler);
+ if (!handler->set)
return -EOPNOTSUPP;
- return inode->i_op->removexattr(dentry, name);
+ return handler->set(handler, dentry, name, NULL, 0, XATTR_REPLACE);
}
EXPORT_SYMBOL(__vfs_removexattr);
@@ -659,61 +729,6 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
return error;
}
-
-static const char *
-strcmp_prefix(const char *a, const char *a_prefix)
-{
- while (*a_prefix && *a == *a_prefix) {
- a++;
- a_prefix++;
- }
- return *a_prefix ? NULL : a;
-}
-
-/*
- * In order to implement different sets of xattr operations for each xattr
- * prefix with the generic xattr API, a filesystem should create a
- * null-terminated array of struct xattr_handler (one for each prefix) and
- * hang a pointer to it off of the xattr field of the inode operations or
- * the s_xattr field of the superblock.
- *
- * The generic_fooxattr() functions will use this list to dispatch xattr
- * operations to the correct xattr_handler.
- */
-#define for_each_xattr_handler(handlers, handler) \
- for ((handler) = *(handlers)++; \
- (handler) != NULL; \
- (handler) = *(handlers)++)
-
-/*
- * Find the xattr_handler with the matching prefix.
- */
-static const struct xattr_handler *
-xattr_resolve_name(const struct dentry *dentry, const char **name)
-{
- const struct xattr_handler *handler, **handlers;
-
- if (!*name)
- return NULL;
-
- handlers = xattr_handlers(dentry->d_inode);
- for_each_xattr_handler(handlers, handler) {
- const char *n;
-
- n = strcmp_prefix(*name, xattr_prefix(handler));
- if (n) {
- if (!handler->prefix ^ !*n) {
- if (*n)
- continue;
- return ERR_PTR(-EINVAL);
- }
- *name = n;
- return handler;
- }
- }
- return ERR_PTR(-EOPNOTSUPP);
-}
-
/*
* Find the handler for the prefix and dispatch its get() operation.
*/
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 8/8] xattr: Remove generic xattr handlers
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (6 preceding siblings ...)
2016-05-02 22:45 ` [RFC 7/8] xattr: Stop calling {get,set,remove}xattr inode operations Andreas Gruenbacher
@ 2016-05-02 22:45 ` Andreas Gruenbacher
2016-05-15 15:10 ` Al Viro
2016-05-02 23:23 ` [RFC 0/8] Xattr inode operation removal Andreas Dilger
2016-05-03 2:40 ` [RFC 0/8] Xattr inode operation removal Mimi Zohar
9 siblings, 1 reply; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-02 22:45 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler
The generic xattr handlers are now unused; remove them.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/9p/vfs_inode_dotl.c | 9 ---------
fs/bad_inode.c | 3 ---
fs/btrfs/inode.c | 12 -----------
fs/ceph/dir.c | 3 ---
fs/ceph/inode.c | 6 ------
fs/cifs/cifsfs.c | 9 ---------
fs/ecryptfs/inode.c | 9 ---------
fs/ext2/file.c | 3 ---
fs/ext2/namei.c | 6 ------
fs/ext2/symlink.c | 6 ------
fs/ext4/file.c | 3 ---
fs/ext4/namei.c | 6 ------
fs/ext4/symlink.c | 9 ---------
fs/f2fs/file.c | 3 ---
fs/f2fs/namei.c | 12 -----------
fs/fuse/dir.c | 9 ---------
fs/gfs2/inode.c | 9 ---------
fs/hfs/inode.c | 2 --
fs/hfsplus/dir.c | 3 ---
fs/hfsplus/inode.c | 3 ---
fs/jffs2/dir.c | 3 ---
fs/jffs2/file.c | 3 ---
fs/jffs2/symlink.c | 3 ---
fs/jfs/file.c | 3 ---
fs/jfs/namei.c | 3 ---
fs/jfs/symlink.c | 6 ------
fs/kernfs/dir.c | 3 ---
fs/kernfs/inode.c | 3 ---
fs/kernfs/symlink.c | 3 ---
fs/libfs.c | 3 ---
fs/nfs/nfs3proc.c | 6 ------
fs/nfs/nfs4proc.c | 6 ------
fs/ocfs2/file.c | 3 ---
fs/ocfs2/namei.c | 3 ---
fs/ocfs2/symlink.c | 3 ---
fs/orangefs/inode.c | 3 ---
fs/orangefs/namei.c | 3 ---
fs/orangefs/symlink.c | 1 -
fs/overlayfs/dir.c | 3 ---
fs/overlayfs/inode.c | 6 ------
fs/reiserfs/file.c | 3 ---
fs/reiserfs/namei.c | 9 ---------
fs/squashfs/inode.c | 1 -
fs/squashfs/namei.c | 1 -
fs/squashfs/symlink.c | 1 -
fs/squashfs/xattr.h | 1 -
fs/ubifs/dir.c | 3 ---
fs/ubifs/file.c | 6 ------
fs/xattr.c | 54 --------------------------------------------------
fs/xfs/xfs_iops.c | 12 -----------
include/linux/fs.h | 4 ----
include/linux/xattr.h | 3 ---
mm/shmem.c | 15 --------------
net/socket.c | 1 -
54 files changed, 309 deletions(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index a34702c..f0f36b8 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -973,9 +973,6 @@ const struct inode_operations v9fs_dir_inode_operations_dotl = {
.rename = v9fs_vfs_rename,
.getattr = v9fs_vfs_getattr_dotl,
.setattr = v9fs_vfs_setattr_dotl,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = v9fs_listxattr,
.get_acl = v9fs_iop_get_acl,
};
@@ -983,9 +980,6 @@ const struct inode_operations v9fs_dir_inode_operations_dotl = {
const struct inode_operations v9fs_file_inode_operations_dotl = {
.getattr = v9fs_vfs_getattr_dotl,
.setattr = v9fs_vfs_setattr_dotl,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = v9fs_listxattr,
.get_acl = v9fs_iop_get_acl,
};
@@ -995,8 +989,5 @@ const struct inode_operations v9fs_symlink_inode_operations_dotl = {
.get_link = v9fs_vfs_get_link_dotl,
.getattr = v9fs_vfs_getattr_dotl,
.setattr = v9fs_vfs_setattr_dotl,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = v9fs_listxattr,
};
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index e9227ea2d..334144c 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -151,10 +151,7 @@ static const struct inode_operations bad_inode_ops =
.permission = bad_inode_permission,
.getattr = bad_inode_getattr,
.setattr = bad_inode_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = bad_inode_listxattr,
- .removexattr = generic_removexattr,
.xattr = bad_inode_xattr_handlers,
};
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 0077b3b..a1090e3 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10160,10 +10160,7 @@ static const struct inode_operations btrfs_dir_inode_operations = {
.symlink = btrfs_symlink,
.setattr = btrfs_setattr,
.mknod = btrfs_mknod,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = btrfs_listxattr,
- .removexattr = generic_removexattr,
.permission = btrfs_permission,
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
@@ -10237,10 +10234,7 @@ static const struct address_space_operations btrfs_symlink_aops = {
static const struct inode_operations btrfs_file_inode_operations = {
.getattr = btrfs_getattr,
.setattr = btrfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = btrfs_listxattr,
- .removexattr = generic_removexattr,
.permission = btrfs_permission,
.fiemap = btrfs_fiemap,
.get_acl = btrfs_get_acl,
@@ -10251,10 +10245,7 @@ static const struct inode_operations btrfs_special_inode_operations = {
.getattr = btrfs_getattr,
.setattr = btrfs_setattr,
.permission = btrfs_permission,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = btrfs_listxattr,
- .removexattr = generic_removexattr,
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.update_time = btrfs_update_time,
@@ -10265,10 +10256,7 @@ static const struct inode_operations btrfs_symlink_inode_operations = {
.getattr = btrfs_getattr,
.setattr = btrfs_setattr,
.permission = btrfs_permission,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = btrfs_listxattr,
- .removexattr = generic_removexattr,
.update_time = btrfs_update_time,
};
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 2e0afdc..ed5753e 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1343,10 +1343,7 @@ const struct inode_operations ceph_dir_iops = {
.permission = ceph_permission,
.getattr = ceph_getattr,
.setattr = ceph_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ceph_listxattr,
- .removexattr = generic_removexattr,
.get_acl = ceph_get_acl,
.set_acl = ceph_set_acl,
.mknod = ceph_mknod,
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index addf8ef..aa64564 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -93,10 +93,7 @@ const struct inode_operations ceph_file_iops = {
.permission = ceph_permission,
.setattr = ceph_setattr,
.getattr = ceph_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ceph_listxattr,
- .removexattr = generic_removexattr,
.get_acl = ceph_get_acl,
.set_acl = ceph_set_acl,
};
@@ -1771,10 +1768,7 @@ static const struct inode_operations ceph_symlink_iops = {
.get_link = simple_get_link,
.setattr = ceph_setattr,
.getattr = ceph_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ceph_listxattr,
- .removexattr = generic_removexattr,
};
int __ceph_setattr(struct inode *inode, struct iattr *attr)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 43cc522b..45d1e4c 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -894,10 +894,7 @@ const struct inode_operations cifs_dir_inode_ops = {
.setattr = cifs_setattr,
.symlink = cifs_symlink,
.mknod = cifs_mknod,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = cifs_listxattr,
- .removexattr = generic_removexattr,
};
const struct inode_operations cifs_file_inode_ops = {
@@ -905,10 +902,7 @@ const struct inode_operations cifs_file_inode_ops = {
.setattr = cifs_setattr,
.getattr = cifs_getattr, /* do we need this anymore? */
.permission = cifs_permission,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = cifs_listxattr,
- .removexattr = generic_removexattr,
};
const struct inode_operations cifs_symlink_inode_ops = {
@@ -918,10 +912,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
/* BB add the following two eventually */
/* revalidate: cifs_revalidate,
setattr: cifs_notify_change, *//* BB do we need notify change */
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = cifs_listxattr,
- .removexattr = generic_removexattr,
};
static int cifs_clone_file_range(struct file *src_file, loff_t off,
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index f8729cb..cb7231f 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1083,10 +1083,7 @@ const struct inode_operations ecryptfs_symlink_iops = {
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
.getattr = ecryptfs_getattr_link,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = generic_removexattr
};
const struct inode_operations ecryptfs_dir_iops = {
@@ -1101,20 +1098,14 @@ const struct inode_operations ecryptfs_dir_iops = {
.rename = ecryptfs_rename,
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = generic_removexattr
};
const struct inode_operations ecryptfs_main_iops = {
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
.getattr = ecryptfs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ecryptfs_listxattr,
- .removexattr = generic_removexattr
};
static int ecryptfs_xattr_get(const struct xattr_handler *handler,
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index c1400b1..8c28f44 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -178,10 +178,7 @@ const struct file_operations ext2_file_operations = {
const struct inode_operations ext2_file_inode_operations = {
#ifdef CONFIG_EXT2_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
- .removexattr = generic_removexattr,
#endif
.setattr = ext2_setattr,
.get_acl = ext2_get_acl,
diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index 1a7eb49..d4d79f2 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -428,10 +428,7 @@ const struct inode_operations ext2_dir_inode_operations = {
.mknod = ext2_mknod,
.rename = ext2_rename,
#ifdef CONFIG_EXT2_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
- .removexattr = generic_removexattr,
#endif
.setattr = ext2_setattr,
.get_acl = ext2_get_acl,
@@ -441,10 +438,7 @@ const struct inode_operations ext2_dir_inode_operations = {
const struct inode_operations ext2_special_inode_operations = {
#ifdef CONFIG_EXT2_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
- .removexattr = generic_removexattr,
#endif
.setattr = ext2_setattr,
.get_acl = ext2_get_acl,
diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
index 3495d8a..8437b19 100644
--- a/fs/ext2/symlink.c
+++ b/fs/ext2/symlink.c
@@ -25,10 +25,7 @@ const struct inode_operations ext2_symlink_inode_operations = {
.get_link = page_get_link,
.setattr = ext2_setattr,
#ifdef CONFIG_EXT2_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -37,9 +34,6 @@ const struct inode_operations ext2_fast_symlink_inode_operations = {
.get_link = simple_get_link,
.setattr = ext2_setattr,
#ifdef CONFIG_EXT2_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
- .removexattr = generic_removexattr,
#endif
};
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 6659e21..e497d4e 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -712,10 +712,7 @@ const struct file_operations ext4_file_operations = {
const struct inode_operations ext4_file_inode_operations = {
.setattr = ext4_setattr,
.getattr = ext4_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
.get_acl = ext4_get_acl,
.set_acl = ext4_set_acl,
.fiemap = ext4_fiemap,
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 5611ec9..ad89756 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3882,10 +3882,7 @@ const struct inode_operations ext4_dir_inode_operations = {
.tmpfile = ext4_tmpfile,
.rename2 = ext4_rename2,
.setattr = ext4_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
.get_acl = ext4_get_acl,
.set_acl = ext4_set_acl,
.fiemap = ext4_fiemap,
@@ -3893,10 +3890,7 @@ const struct inode_operations ext4_dir_inode_operations = {
const struct inode_operations ext4_special_inode_operations = {
.setattr = ext4_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
.get_acl = ext4_get_acl,
.set_acl = ext4_set_acl,
};
diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
index 6f7ee30..61f440d 100644
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@ -94,10 +94,7 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = ext4_encrypted_get_link,
.setattr = ext4_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
};
#endif
@@ -105,18 +102,12 @@ const struct inode_operations ext4_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = page_get_link,
.setattr = ext4_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
};
const struct inode_operations ext4_fast_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = simple_get_link,
.setattr = ext4_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ext4_listxattr,
- .removexattr = generic_removexattr,
};
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b41c357..54494c2 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -725,10 +725,7 @@ const struct inode_operations f2fs_file_inode_operations = {
.get_acl = f2fs_get_acl,
.set_acl = f2fs_set_acl,
#ifdef CONFIG_F2FS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = f2fs_listxattr,
- .removexattr = generic_removexattr,
#endif
.fiemap = f2fs_fiemap,
};
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index db874ad..5b20a9a 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1066,10 +1066,7 @@ const struct inode_operations f2fs_encrypted_symlink_inode_operations = {
.getattr = f2fs_getattr,
.setattr = f2fs_setattr,
#ifdef CONFIG_F2FS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = f2fs_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -1089,10 +1086,7 @@ const struct inode_operations f2fs_dir_inode_operations = {
.get_acl = f2fs_get_acl,
.set_acl = f2fs_set_acl,
#ifdef CONFIG_F2FS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = f2fs_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -1102,10 +1096,7 @@ const struct inode_operations f2fs_symlink_inode_operations = {
.getattr = f2fs_getattr,
.setattr = f2fs_setattr,
#ifdef CONFIG_F2FS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = f2fs_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -1115,9 +1106,6 @@ const struct inode_operations f2fs_special_inode_operations = {
.get_acl = f2fs_get_acl,
.set_acl = f2fs_set_acl,
#ifdef CONFIG_F2FS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = f2fs_listxattr,
- .removexattr = generic_removexattr,
#endif
};
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c36b3a3..dcb16a4 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1884,10 +1884,7 @@ static const struct inode_operations fuse_dir_inode_operations = {
.mknod = fuse_mknod,
.permission = fuse_permission,
.getattr = fuse_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = generic_removexattr,
};
static const struct file_operations fuse_dir_operations = {
@@ -1905,10 +1902,7 @@ static const struct inode_operations fuse_common_inode_operations = {
.setattr = fuse_setattr,
.permission = fuse_permission,
.getattr = fuse_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = generic_removexattr,
};
static const struct inode_operations fuse_symlink_inode_operations = {
@@ -1916,10 +1910,7 @@ static const struct inode_operations fuse_symlink_inode_operations = {
.get_link = fuse_get_link,
.readlink = generic_readlink,
.getattr = fuse_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = fuse_listxattr,
- .removexattr = generic_removexattr,
};
void fuse_init_common(struct inode *inode)
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 3398342..3ef6938 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1994,10 +1994,7 @@ const struct inode_operations gfs2_file_iops = {
.permission = gfs2_permission,
.setattr = gfs2_setattr,
.getattr = gfs2_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = gfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = gfs2_fiemap,
.get_acl = gfs2_get_acl,
.set_acl = gfs2_set_acl,
@@ -2016,10 +2013,7 @@ const struct inode_operations gfs2_dir_iops = {
.permission = gfs2_permission,
.setattr = gfs2_setattr,
.getattr = gfs2_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = gfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = gfs2_fiemap,
.get_acl = gfs2_get_acl,
.set_acl = gfs2_set_acl,
@@ -2032,10 +2026,7 @@ const struct inode_operations gfs2_symlink_iops = {
.permission = gfs2_permission,
.setattr = gfs2_setattr,
.getattr = gfs2_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = gfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = gfs2_fiemap,
};
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index bb9549b..4b7a510 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -687,7 +687,5 @@ static const struct file_operations hfs_file_operations = {
static const struct inode_operations hfs_file_inode_operations = {
.lookup = hfs_file_lookup,
.setattr = hfs_inode_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = generic_listxattr,
};
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index a4e867e..5ff4e29 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -556,10 +556,7 @@ const struct inode_operations hfsplus_dir_inode_operations = {
.symlink = hfsplus_symlink,
.mknod = hfsplus_mknod,
.rename = hfsplus_rename,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = hfsplus_listxattr,
- .removexattr = generic_removexattr,
#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
.get_acl = hfsplus_get_posix_acl,
.set_acl = hfsplus_set_posix_acl,
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 1a6394c..4a80a7e1 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -334,10 +334,7 @@ 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,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = hfsplus_listxattr,
- .removexattr = generic_removexattr,
#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
.get_acl = hfsplus_get_posix_acl,
.set_acl = hfsplus_set_posix_acl,
diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index 9130c7a..3779744 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -61,10 +61,7 @@ const struct inode_operations jffs2_dir_inode_operations =
.get_acl = jffs2_get_acl,
.set_acl = jffs2_set_acl,
.setattr = jffs2_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jffs2_listxattr,
- .removexattr = generic_removexattr
};
/***********************************************************************/
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index bf543cb..5817c25 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -66,10 +66,7 @@ const struct inode_operations jffs2_file_inode_operations =
.get_acl = jffs2_get_acl,
.set_acl = jffs2_set_acl,
.setattr = jffs2_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jffs2_listxattr,
- .removexattr = generic_removexattr
};
const struct address_space_operations jffs2_file_address_operations =
diff --git a/fs/jffs2/symlink.c b/fs/jffs2/symlink.c
index afe2d75..8f3f085 100644
--- a/fs/jffs2/symlink.c
+++ b/fs/jffs2/symlink.c
@@ -16,8 +16,5 @@ const struct inode_operations jffs2_symlink_inode_operations =
.readlink = generic_readlink,
.get_link = simple_get_link,
.setattr = jffs2_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jffs2_listxattr,
- .removexattr = generic_removexattr
};
diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 7f1a585..f6eb041 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -140,10 +140,7 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr)
}
const struct inode_operations jfs_file_inode_operations = {
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jfs_listxattr,
- .removexattr = generic_removexattr,
.setattr = jfs_setattr,
#ifdef CONFIG_JFS_POSIX_ACL
.get_acl = jfs_get_acl,
diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c
index 7040062..e4c944b 100644
--- a/fs/jfs/namei.c
+++ b/fs/jfs/namei.c
@@ -1537,10 +1537,7 @@ const struct inode_operations jfs_dir_inode_operations = {
.rmdir = jfs_rmdir,
.mknod = jfs_mknod,
.rename = jfs_rename,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jfs_listxattr,
- .removexattr = generic_removexattr,
.setattr = jfs_setattr,
#ifdef CONFIG_JFS_POSIX_ACL
.get_acl = jfs_get_acl,
diff --git a/fs/jfs/symlink.c b/fs/jfs/symlink.c
index c94c7e4..c82404f 100644
--- a/fs/jfs/symlink.c
+++ b/fs/jfs/symlink.c
@@ -25,19 +25,13 @@ const struct inode_operations jfs_fast_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = simple_get_link,
.setattr = jfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jfs_listxattr,
- .removexattr = generic_removexattr,
};
const struct inode_operations jfs_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = page_get_link,
.setattr = jfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = jfs_listxattr,
- .removexattr = generic_removexattr,
};
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index f67d896..af660a7 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1125,9 +1125,6 @@ const struct inode_operations kernfs_dir_iops = {
.permission = kernfs_iop_permission,
.setattr = kernfs_iop_setattr,
.getattr = kernfs_iop_getattr,
- .setxattr = generic_setxattr,
- .removexattr = generic_removexattr,
- .getxattr = generic_getxattr,
.listxattr = kernfs_iop_listxattr,
.mkdir = kernfs_iop_mkdir,
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 0f13396..9ee7b70 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -28,9 +28,6 @@ static const struct inode_operations kernfs_iops = {
.permission = kernfs_iop_permission,
.setattr = kernfs_iop_setattr,
.getattr = kernfs_iop_getattr,
- .setxattr = generic_setxattr,
- .removexattr = generic_removexattr,
- .getxattr = generic_getxattr,
.listxattr = kernfs_iop_listxattr,
};
diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c
index 549a14c7..9b43ca0 100644
--- a/fs/kernfs/symlink.c
+++ b/fs/kernfs/symlink.c
@@ -134,9 +134,6 @@ static const char *kernfs_iop_get_link(struct dentry *dentry,
}
const struct inode_operations kernfs_symlink_iops = {
- .setxattr = generic_setxattr,
- .removexattr = generic_removexattr,
- .getxattr = generic_getxattr,
.listxattr = kernfs_iop_listxattr,
.readlink = generic_readlink,
.get_link = kernfs_iop_get_link,
diff --git a/fs/libfs.c b/fs/libfs.c
index a7cea0a..11b49bf 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1137,9 +1137,6 @@ static const struct inode_operations empty_dir_inode_operations = {
.permission = generic_permission,
.setattr = empty_dir_setattr,
.getattr = empty_dir_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = empty_dir_listxattr,
.xattr = empty_dir_xattr_handlers,
};
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index cb28cce..0be4e42 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -899,9 +899,6 @@ static const struct inode_operations nfs3_dir_inode_operations = {
.setattr = nfs_setattr,
#ifdef CONFIG_NFS_V3_ACL
.listxattr = nfs3_listxattr,
- .getxattr = generic_getxattr,
- .setxattr = generic_setxattr,
- .removexattr = generic_removexattr,
.get_acl = nfs3_get_acl,
.set_acl = nfs3_set_acl,
#endif
@@ -913,9 +910,6 @@ static const struct inode_operations nfs3_file_inode_operations = {
.setattr = nfs_setattr,
#ifdef CONFIG_NFS_V3_ACL
.listxattr = nfs3_listxattr,
- .getxattr = generic_getxattr,
- .setxattr = generic_setxattr,
- .removexattr = generic_removexattr,
.get_acl = nfs3_get_acl,
.set_acl = nfs3_set_acl,
#endif
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7a7ac1d..48b970d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8853,20 +8853,14 @@ static const struct inode_operations nfs4_dir_inode_operations = {
.permission = nfs_permission,
.getattr = nfs_getattr,
.setattr = nfs_setattr,
- .getxattr = generic_getxattr,
- .setxattr = generic_setxattr,
.listxattr = nfs4_listxattr,
- .removexattr = generic_removexattr,
};
static const struct inode_operations nfs4_file_inode_operations = {
.permission = nfs_permission,
.getattr = nfs_getattr,
.setattr = nfs_setattr,
- .getxattr = generic_getxattr,
- .setxattr = generic_setxattr,
.listxattr = nfs4_listxattr,
- .removexattr = generic_removexattr,
};
const struct nfs_rpc_ops nfs_v4_clientops = {
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index c6fdcbd..0200a4f 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2460,10 +2460,7 @@ const struct inode_operations ocfs2_file_iops = {
.setattr = ocfs2_setattr,
.getattr = ocfs2_getattr,
.permission = ocfs2_permission,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ocfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = ocfs2_fiemap,
.get_acl = ocfs2_iop_get_acl,
.set_acl = ocfs2_iop_set_acl,
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 6b3e871..24eaca1 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -2932,10 +2932,7 @@ const struct inode_operations ocfs2_dir_iops = {
.setattr = ocfs2_setattr,
.getattr = ocfs2_getattr,
.permission = ocfs2_permission,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ocfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = ocfs2_fiemap,
.get_acl = ocfs2_iop_get_acl,
.set_acl = ocfs2_iop_set_acl,
diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
index 6c2a3e3..6ad8eec 100644
--- a/fs/ocfs2/symlink.c
+++ b/fs/ocfs2/symlink.c
@@ -91,9 +91,6 @@ const struct inode_operations ocfs2_symlink_inode_operations = {
.get_link = page_get_link,
.getattr = ocfs2_getattr,
.setattr = ocfs2_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ocfs2_listxattr,
- .removexattr = generic_removexattr,
.fiemap = ocfs2_fiemap,
};
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 2382e26..a4d26c1 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -313,10 +313,7 @@ struct inode_operations orangefs_file_inode_operations = {
.set_acl = orangefs_set_acl,
.setattr = orangefs_setattr,
.getattr = orangefs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = orangefs_listxattr,
- .removexattr = generic_removexattr,
.permission = orangefs_permission,
};
diff --git a/fs/orangefs/namei.c b/fs/orangefs/namei.c
index 5a60c50..d396d03 100644
--- a/fs/orangefs/namei.c
+++ b/fs/orangefs/namei.c
@@ -454,9 +454,6 @@ struct inode_operations orangefs_dir_inode_operations = {
.rename = orangefs_rename,
.setattr = orangefs_setattr,
.getattr = orangefs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = orangefs_listxattr,
.permission = orangefs_permission,
};
diff --git a/fs/orangefs/symlink.c b/fs/orangefs/symlink.c
index 6418dd6..dade1ef 100644
--- a/fs/orangefs/symlink.c
+++ b/fs/orangefs/symlink.c
@@ -14,6 +14,5 @@ struct inode_operations orangefs_symlink_inode_operations = {
.setattr = orangefs_setattr,
.getattr = orangefs_getattr,
.listxattr = orangefs_listxattr,
- .setxattr = generic_setxattr,
.permission = orangefs_permission,
};
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 16ba0f8..6438002 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -967,8 +967,5 @@ const struct inode_operations ovl_dir_inode_operations = {
.mknod = ovl_mknod,
.permission = ovl_permission,
.getattr = ovl_dir_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = generic_removexattr,
};
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index d08c926..10dda4b 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -374,10 +374,7 @@ static const struct inode_operations ovl_file_inode_operations = {
.setattr = ovl_setattr,
.permission = ovl_permission,
.getattr = ovl_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = generic_removexattr,
};
static const struct inode_operations ovl_symlink_inode_operations = {
@@ -385,10 +382,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
.get_link = ovl_get_link,
.readlink = ovl_readlink,
.getattr = ovl_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ovl_listxattr,
- .removexattr = generic_removexattr,
};
struct inode *ovl_new_inode(struct super_block *sb, umode_t mode,
diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c
index 9ffaf71..6574a03 100644
--- a/fs/reiserfs/file.c
+++ b/fs/reiserfs/file.c
@@ -260,10 +260,7 @@ const struct file_operations reiserfs_file_operations = {
const struct inode_operations reiserfs_file_inode_operations = {
.setattr = reiserfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = reiserfs_listxattr,
- .removexattr = generic_removexattr,
.permission = reiserfs_permission,
.get_acl = reiserfs_get_acl,
.set_acl = reiserfs_set_acl,
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 8a36696..fd7d060 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -1650,10 +1650,7 @@ const struct inode_operations reiserfs_dir_inode_operations = {
.mknod = reiserfs_mknod,
.rename = reiserfs_rename,
.setattr = reiserfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = reiserfs_listxattr,
- .removexattr = generic_removexattr,
.permission = reiserfs_permission,
.get_acl = reiserfs_get_acl,
.set_acl = reiserfs_set_acl,
@@ -1667,10 +1664,7 @@ const struct inode_operations reiserfs_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = page_get_link,
.setattr = reiserfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = reiserfs_listxattr,
- .removexattr = generic_removexattr,
.permission = reiserfs_permission,
};
@@ -1679,10 +1673,7 @@ const struct inode_operations reiserfs_symlink_inode_operations = {
*/
const struct inode_operations reiserfs_special_inode_operations = {
.setattr = reiserfs_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = reiserfs_listxattr,
- .removexattr = generic_removexattr,
.permission = reiserfs_permission,
.get_acl = reiserfs_get_acl,
.set_acl = reiserfs_set_acl,
diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c
index 0927b1e..e9793b1 100644
--- a/fs/squashfs/inode.c
+++ b/fs/squashfs/inode.c
@@ -425,7 +425,6 @@ failed_read:
const struct inode_operations squashfs_inode_ops = {
- .getxattr = generic_getxattr,
.listxattr = squashfs_listxattr
};
diff --git a/fs/squashfs/namei.c b/fs/squashfs/namei.c
index 67cad77..40c10d9 100644
--- a/fs/squashfs/namei.c
+++ b/fs/squashfs/namei.c
@@ -247,6 +247,5 @@ failed:
const struct inode_operations squashfs_dir_inode_ops = {
.lookup = squashfs_lookup,
- .getxattr = generic_getxattr,
.listxattr = squashfs_listxattr
};
diff --git a/fs/squashfs/symlink.c b/fs/squashfs/symlink.c
index dbcc2f5..637d7e1 100644
--- a/fs/squashfs/symlink.c
+++ b/fs/squashfs/symlink.c
@@ -120,7 +120,6 @@ const struct address_space_operations squashfs_symlink_aops = {
const struct inode_operations squashfs_symlink_inode_ops = {
.readlink = generic_readlink,
.get_link = page_get_link,
- .getxattr = generic_getxattr,
.listxattr = squashfs_listxattr
};
diff --git a/fs/squashfs/xattr.h b/fs/squashfs/xattr.h
index c83f5d9..afe70f8 100644
--- a/fs/squashfs/xattr.h
+++ b/fs/squashfs/xattr.h
@@ -42,6 +42,5 @@ static inline int squashfs_xattr_lookup(struct super_block *sb,
return 0;
}
#define squashfs_listxattr NULL
-#define generic_getxattr NULL
#define squashfs_xattr_handlers NULL
#endif
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 4d07d15..2d75a9f 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1182,10 +1182,7 @@ const struct inode_operations ubifs_dir_inode_operations = {
.rename = ubifs_rename,
.setattr = ubifs_setattr,
.getattr = ubifs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ubifs_listxattr,
- .removexattr = generic_removexattr,
#ifdef CONFIG_UBIFS_ATIME_SUPPORT
.update_time = ubifs_update_time,
#endif
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index e149437..2f1df7e 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1597,10 +1597,7 @@ const struct address_space_operations ubifs_file_address_operations = {
const struct inode_operations ubifs_file_inode_operations = {
.setattr = ubifs_setattr,
.getattr = ubifs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ubifs_listxattr,
- .removexattr = generic_removexattr,
#ifdef CONFIG_UBIFS_ATIME_SUPPORT
.update_time = ubifs_update_time,
#endif
@@ -1611,10 +1608,7 @@ const struct inode_operations ubifs_symlink_inode_operations = {
.get_link = simple_get_link,
.setattr = ubifs_setattr,
.getattr = ubifs_getattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = ubifs_listxattr,
- .removexattr = generic_removexattr,
#ifdef CONFIG_UBIFS_ATIME_SUPPORT
.update_time = ubifs_update_time,
#endif
diff --git a/fs/xattr.c b/fs/xattr.c
index a3fa123..48b8d5e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -40,9 +40,6 @@ strcmp_prefix(const char *a, const char *a_prefix)
* null-terminated array of struct xattr_handler (one for each prefix) and
* hang a pointer to it off of the xattr field of the inode operations or
* the s_xattr field of the superblock.
- *
- * The generic_fooxattr() functions will use this list to dispatch xattr
- * operations to the correct xattr_handler.
*/
#define for_each_xattr_handler(handlers, handler) \
for ((handler) = *(handlers)++; \
@@ -730,22 +727,6 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
}
/*
- * Find the handler for the prefix and dispatch its get() operation.
- */
-ssize_t
-generic_getxattr(struct dentry *dentry, struct inode *inode,
- const char *name, void *buffer, size_t size)
-{
- const struct xattr_handler *handler;
-
- handler = xattr_resolve_name(dentry, &name);
- if (IS_ERR(handler))
- return PTR_ERR(handler);
- return handler->get(handler, dentry, inode,
- name, buffer, size);
-}
-
-/*
* Combine the results of the list() operation from every xattr_handler in the
* list.
*/
@@ -782,42 +763,7 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
}
return size;
}
-
-/*
- * Find the handler for the prefix and dispatch its set() operation.
- */
-int
-generic_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags)
-{
- const struct xattr_handler *handler;
-
- if (size == 0)
- value = ""; /* empty EA, do not remove */
- handler = xattr_resolve_name(dentry, &name);
- if (IS_ERR(handler))
- return PTR_ERR(handler);
- return handler->set(handler, dentry, name, value, size, flags);
-}
-
-/*
- * Find the handler for the prefix and dispatch its set() operation to remove
- * any associated extended attribute.
- */
-int
-generic_removexattr(struct dentry *dentry, const char *name)
-{
- const struct xattr_handler *handler;
-
- handler = xattr_resolve_name(dentry, &name);
- if (IS_ERR(handler))
- return PTR_ERR(handler);
- return handler->set(handler, dentry, name, NULL, 0, XATTR_REPLACE);
-}
-
-EXPORT_SYMBOL(generic_getxattr);
EXPORT_SYMBOL(generic_listxattr);
-EXPORT_SYMBOL(generic_setxattr);
-EXPORT_SYMBOL(generic_removexattr);
/**
* xattr_full_name - Compute full attribute name from suffix
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index fb7dc61..3d3a589 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1091,9 +1091,6 @@ static const struct inode_operations xfs_inode_operations = {
.set_acl = xfs_set_acl,
.getattr = xfs_vn_getattr,
.setattr = xfs_vn_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.fiemap = xfs_vn_fiemap,
.update_time = xfs_vn_update_time,
@@ -1119,9 +1116,6 @@ static const struct inode_operations xfs_dir_inode_operations = {
.set_acl = xfs_set_acl,
.getattr = xfs_vn_getattr,
.setattr = xfs_vn_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
.tmpfile = xfs_vn_tmpfile,
@@ -1147,9 +1141,6 @@ static const struct inode_operations xfs_dir_ci_inode_operations = {
.set_acl = xfs_set_acl,
.getattr = xfs_vn_getattr,
.setattr = xfs_vn_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
.tmpfile = xfs_vn_tmpfile,
@@ -1160,9 +1151,6 @@ static const struct inode_operations xfs_symlink_inode_operations = {
.get_link = xfs_vn_get_link,
.getattr = xfs_vn_getattr,
.setattr = xfs_vn_setattr,
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
};
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4f25cd3..d6d186f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1701,11 +1701,7 @@ struct inode_operations {
struct inode *, struct dentry *, unsigned int);
int (*setattr) (struct dentry *, struct iattr *);
int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
- int (*setxattr) (struct dentry *, const char *,const void *,size_t,int);
- ssize_t (*getxattr) (struct dentry *, struct inode *,
- const char *, void *, size_t);
ssize_t (*listxattr) (struct dentry *, char *, size_t);
- int (*removexattr) (struct dentry *, const char *);
int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
u64 len);
int (*update_time)(struct inode *, struct timespec *, int);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 76557ce..38728b0 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -56,10 +56,7 @@ int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
int __vfs_removexattr(struct dentry *, const char *);
int vfs_removexattr(struct dentry *, const char *);
-ssize_t generic_getxattr(struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size);
ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);
-int generic_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags);
-int generic_removexattr(struct dentry *dentry, const char *name);
ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
char **xattr_value, size_t size, gfp_t flags);
diff --git a/mm/shmem.c b/mm/shmem.c
index 00d5d02..7ce1d8a 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2698,10 +2698,7 @@ static const struct inode_operations shmem_short_symlink_operations = {
.readlink = generic_readlink,
.get_link = simple_get_link,
#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = shmem_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -2709,10 +2706,7 @@ static const struct inode_operations shmem_symlink_inode_operations = {
.readlink = generic_readlink,
.get_link = shmem_get_link,
#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = shmem_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -3183,10 +3177,7 @@ static const struct inode_operations shmem_inode_operations = {
.getattr = shmem_getattr,
.setattr = shmem_setattr,
#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = shmem_listxattr,
- .removexattr = generic_removexattr,
.set_acl = simple_set_acl,
#endif
};
@@ -3205,10 +3196,7 @@ static const struct inode_operations shmem_dir_inode_operations = {
.tmpfile = shmem_tmpfile,
#endif
#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = shmem_listxattr,
- .removexattr = generic_removexattr,
#endif
#ifdef CONFIG_TMPFS_POSIX_ACL
.setattr = shmem_setattr,
@@ -3218,10 +3206,7 @@ static const struct inode_operations shmem_dir_inode_operations = {
static const struct inode_operations shmem_special_inode_operations = {
#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
.listxattr = shmem_listxattr,
- .removexattr = generic_removexattr,
#endif
#ifdef CONFIG_TMPFS_POSIX_ACL
.setattr = shmem_setattr,
diff --git a/net/socket.c b/net/socket.c
index 3ef77ab..7976fa8 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -519,7 +519,6 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
}
static const struct inode_operations sockfs_inode_ops = {
- .getxattr = generic_getxattr,
.listxattr = sockfs_listxattr,
};
--
2.5.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 8/8] xattr: Remove generic xattr handlers
2016-05-02 22:45 ` [RFC 8/8] xattr: Remove generic xattr handlers Andreas Gruenbacher
@ 2016-05-15 15:10 ` Al Viro
0 siblings, 0 replies; 23+ messages in thread
From: Al Viro @ 2016-05-15 15:10 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: linux-fsdevel, Tyler Hicks, ecryptfs, Miklos Szeredi,
linux-unionfs, fuse-devel, Mimi Zohar, linux-ima-devel,
linux-security-module, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler
On Tue, May 03, 2016 at 12:45:18AM +0200, Andreas Gruenbacher wrote:
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index bb9549b..4b7a510 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -687,7 +687,5 @@ static const struct file_operations hfs_file_operations = {
> static const struct inode_operations hfs_file_inode_operations = {
> .lookup = hfs_file_lookup,
> .setattr = hfs_inode_setattr,
> - .setxattr = generic_setxattr,
> - .getxattr = generic_getxattr,
Where has that come from? I don't see anything else in your series touching
that file and work.xattr doesn't touch it either. With
static const struct inode_operations hfs_file_inode_operations = {
.lookup = hfs_file_lookup,
.setattr = hfs_inode_setattr,
.setxattr = hfs_setxattr,
.getxattr = hfs_getxattr,
.listxattr = hfs_listxattr,
};
being what's in the tree. And it *is* an interesting one, seeing that this
is one case where some inodes on a given fs do have ->...xattr and some do
not, so I'd like to see the details.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 0/8] Xattr inode operation removal
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (7 preceding siblings ...)
2016-05-02 22:45 ` [RFC 8/8] xattr: Remove generic xattr handlers Andreas Gruenbacher
@ 2016-05-02 23:23 ` Andreas Dilger
2016-05-03 10:38 ` Andreas Gruenbacher
` (2 more replies)
2016-05-03 2:40 ` [RFC 0/8] Xattr inode operation removal Mimi Zohar
9 siblings, 3 replies; 23+ messages in thread
From: Andreas Dilger @ 2016-05-02 23:23 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler, Oleg Drokin, Andreas Dilger,
James Simmons
[-- Attachment #1: Type: text/plain, Size: 5711 bytes --]
On May 2, 2016, at 4:45 PM, Andreas Gruenbacher <agruenba@redhat.com> wrote:
>
> Hi all,
>
> what are your thoughts on this patch set? It applies on top of the
> work.xattr branch [*], converts the remaining filesystems over to xattr
> handlers, and replaces the getxattr, setxattr, and removexattr inode
> operations. The only way to implement getxattr, setxattr, and
> removexattr with this approach is through xattr handlers.
Maybe I'm missing the point of this patch, but is there a long-term benefit
to this change? It seems to be replacing the ->getxattr() and related inode
methods with a generic ->xattr handler list that needs to be demultiplexed
on each access? Is there a net improvement in functionality that comes
out the other end? That isn't at all clear from your comments or the
patches themselves. It seems that all of LOC savings is from deleting
the .setxattr, .getxattr, and .removexattr lines from inode_operations
but there is added complexity in the rest of the code?
Cheers, Andreas
>
> *** Please don't merge yet: this is boot tested only so far! ***
>
> Lustre currently also breaks; I haven't succeeded in cleaning up the
> mess there. Oleg and Andreas, would you like to look into that?
>
> Thanks,
> Andreas
>
> [*] https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git/log/?h=work.xattr
>
> Andreas Gruenbacher (8):
> ecryptfs: Switch to generic xattr handlers
> overlayfs: Switch to generic xattr handlers
> fuse: Switch to generic xattr handlers
> evm: Turn evm_update_evmxattr into void function
> xattr: Add per-inode xattr handlers as a new inode operation
> xattr: Add __vfs_{get,set,remove}xattr helpers
> xattr: Stop calling {get,set,remove}xattr inode operations
> xattr: Remove generic xattr handlers
>
> fs/9p/vfs_inode_dotl.c | 9 --
> fs/bad_inode.c | 33 +++--
> fs/btrfs/inode.c | 12 --
> fs/cachefiles/bind.c | 4 +-
> fs/cachefiles/namei.c | 4 +-
> fs/ceph/dir.c | 3 -
> fs/ceph/inode.c | 6 -
> fs/cifs/cifsfs.c | 9 --
> fs/ecryptfs/ecryptfs_kernel.h | 2 +
> fs/ecryptfs/inode.c | 64 +++++-----
> fs/ecryptfs/main.c | 1 +
> fs/ecryptfs/mmap.c | 15 +--
> fs/ext2/file.c | 3 -
> fs/ext2/namei.c | 6 -
> fs/ext2/symlink.c | 6 -
> fs/ext4/file.c | 3 -
> fs/ext4/namei.c | 6 -
> fs/ext4/symlink.c | 9 --
> fs/f2fs/file.c | 3 -
> fs/f2fs/namei.c | 12 --
> fs/fuse/dir.c | 40 ++++--
> fs/fuse/fuse_i.h | 2 +
> fs/fuse/inode.c | 1 +
> fs/gfs2/inode.c | 9 --
> fs/hfs/inode.c | 2 -
> fs/hfsplus/dir.c | 3 -
> fs/hfsplus/inode.c | 3 -
> fs/jffs2/dir.c | 3 -
> fs/jffs2/file.c | 3 -
> fs/jffs2/symlink.c | 3 -
> fs/jfs/file.c | 3 -
> fs/jfs/namei.c | 3 -
> fs/jfs/symlink.c | 6 -
> fs/kernfs/dir.c | 3 -
> fs/kernfs/inode.c | 3 -
> fs/kernfs/symlink.c | 3 -
> fs/libfs.c | 26 +---
> fs/nfs/nfs3proc.c | 6 -
> fs/nfs/nfs4proc.c | 6 -
> fs/ocfs2/file.c | 3 -
> fs/ocfs2/namei.c | 3 -
> fs/ocfs2/symlink.c | 3 -
> fs/orangefs/inode.c | 3 -
> fs/orangefs/namei.c | 3 -
> fs/orangefs/symlink.c | 1 -
> fs/overlayfs/copy_up.c | 4 -
> fs/overlayfs/dir.c | 3 -
> fs/overlayfs/inode.c | 46 +++++--
> fs/overlayfs/overlayfs.h | 6 +-
> fs/overlayfs/super.c | 5 +-
> fs/reiserfs/file.c | 3 -
> fs/reiserfs/namei.c | 9 --
> fs/squashfs/inode.c | 1 -
> fs/squashfs/namei.c | 1 -
> fs/squashfs/symlink.c | 1 -
> fs/squashfs/xattr.h | 1 -
> fs/ubifs/dir.c | 3 -
> fs/ubifs/file.c | 6 -
> fs/xattr.c | 231 ++++++++++++++++------------------
> fs/xfs/xfs_iops.c | 12 --
> include/linux/fs.h | 5 +-
> include/linux/xattr.h | 12 +-
> mm/shmem.c | 15 ---
> net/socket.c | 1 -
> security/commoncap.c | 25 ++--
> security/integrity/evm/evm.h | 7 +-
> security/integrity/evm/evm_crypto.c | 20 ++-
> security/integrity/evm/evm_main.c | 5 +-
> security/integrity/ima/ima_appraise.c | 23 ++--
> security/selinux/hooks.c | 28 +----
> security/smack/smack_lsm.c | 28 ++---
> 71 files changed, 314 insertions(+), 541 deletions(-)
>
> --
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 0/8] Xattr inode operation removal
2016-05-02 23:23 ` [RFC 0/8] Xattr inode operation removal Andreas Dilger
@ 2016-05-03 10:38 ` Andreas Gruenbacher
2016-05-04 20:13 ` James Simmons
2016-05-11 15:54 ` [PATCH] xattr handlers: fixup generic_listxattr James Simmons
2 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-03 10:38 UTC (permalink / raw)
To: Andreas Dilger
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, LSM, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler, Oleg Drokin, Andreas Dilger, James Simmons
On Tue, May 3, 2016 at 1:23 AM, Andreas Dilger <adilger@dilger.ca> wrote:
> On May 2, 2016, at 4:45 PM, Andreas Gruenbacher <agruenba@redhat.com> wrote:
>>
>> Hi all,
>>
>> what are your thoughts on this patch set? It applies on top of the
>> work.xattr branch [*], converts the remaining filesystems over to xattr
>> handlers, and replaces the getxattr, setxattr, and removexattr inode
>> operations. The only way to implement getxattr, setxattr, and
>> removexattr with this approach is through xattr handlers.
>
> Maybe I'm missing the point of this patch, but is there a long-term benefit
> to this change? It seems to be replacing the ->getxattr() and related inode
> methods with a generic ->xattr handler list that needs to be demultiplexed
> on each access? Is there a net improvement in functionality that comes
> out the other end?
All filesystems except for things like overlayfs and fuse implement this kind of
multiplexing already, so we might as well make that the default.
I'm hoping that we can further clean things up on top to get rid of at
least some
of the name parsing in fs/xattr.c that has crept in over time; it's not pretty.
> That isn't at all clear from your comments or the
> patches themselves. It seems that all of LOC savings is from deleting
> the .setxattr, .getxattr, and .removexattr lines from inode_operations
> but there is added complexity in the rest of the code?
Hmm, except for the rather trivial dummy dispatching for bad inodes, "empty"
directories, fuse, and overlayfs, here do you see increased complexity? The
indirection through the generic inode operations goes away, which isn't a bad
thing.
I agree that better documentation would help.
Thanks,
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 0/8] Xattr inode operation removal
2016-05-02 23:23 ` [RFC 0/8] Xattr inode operation removal Andreas Dilger
2016-05-03 10:38 ` Andreas Gruenbacher
@ 2016-05-04 20:13 ` James Simmons
2016-05-11 15:54 ` [PATCH] xattr handlers: fixup generic_listxattr James Simmons
2 siblings, 0 replies; 23+ messages in thread
From: James Simmons @ 2016-05-04 20:13 UTC (permalink / raw)
To: Andreas Dilger
Cc: Andreas Gruenbacher, Alexander Viro, linux-fsdevel, Tyler Hicks,
ecryptfs, Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler, Oleg Drokin, Andreas Dilger
> On May 2, 2016, at 4:45 PM, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> >
> > Hi all,
> >
> > what are your thoughts on this patch set? It applies on top of the
> > work.xattr branch [*], converts the remaining filesystems over to xattr
> > handlers, and replaces the getxattr, setxattr, and removexattr inode
> > operations. The only way to implement getxattr, setxattr, and
> > removexattr with this approach is through xattr handlers.
>
> Maybe I'm missing the point of this patch, but is there a long-term benefit
> to this change? It seems to be replacing the ->getxattr() and related inode
> methods with a generic ->xattr handler list that needs to be demultiplexed
> on each access? Is there a net improvement in functionality that comes
> out the other end? That isn't at all clear from your comments or the
> patches themselves. It seems that all of LOC savings is from deleting
> the .setxattr, .getxattr, and .removexattr lines from inode_operations
> but there is added complexity in the rest of the code?
>
> Cheers, Andreas
>
> >
> > *** Please don't merge yet: this is boot tested only so far! ***
> >
> > Lustre currently also breaks; I haven't succeeded in cleaning up the
> > mess there. Oleg and Andreas, would you like to look into that?
Al do you plan on merging this for 4.7-rc1?
> > Thanks,
> > Andreas
> >
> > [*] https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git/log/?h=work.xattr
> >
> > Andreas Gruenbacher (8):
> > ecryptfs: Switch to generic xattr handlers
> > overlayfs: Switch to generic xattr handlers
> > fuse: Switch to generic xattr handlers
> > evm: Turn evm_update_evmxattr into void function
> > xattr: Add per-inode xattr handlers as a new inode operation
> > xattr: Add __vfs_{get,set,remove}xattr helpers
> > xattr: Stop calling {get,set,remove}xattr inode operations
> > xattr: Remove generic xattr handlers
> >
> > fs/9p/vfs_inode_dotl.c | 9 --
> > fs/bad_inode.c | 33 +++--
> > fs/btrfs/inode.c | 12 --
> > fs/cachefiles/bind.c | 4 +-
> > fs/cachefiles/namei.c | 4 +-
> > fs/ceph/dir.c | 3 -
> > fs/ceph/inode.c | 6 -
> > fs/cifs/cifsfs.c | 9 --
> > fs/ecryptfs/ecryptfs_kernel.h | 2 +
> > fs/ecryptfs/inode.c | 64 +++++-----
> > fs/ecryptfs/main.c | 1 +
> > fs/ecryptfs/mmap.c | 15 +--
> > fs/ext2/file.c | 3 -
> > fs/ext2/namei.c | 6 -
> > fs/ext2/symlink.c | 6 -
> > fs/ext4/file.c | 3 -
> > fs/ext4/namei.c | 6 -
> > fs/ext4/symlink.c | 9 --
> > fs/f2fs/file.c | 3 -
> > fs/f2fs/namei.c | 12 --
> > fs/fuse/dir.c | 40 ++++--
> > fs/fuse/fuse_i.h | 2 +
> > fs/fuse/inode.c | 1 +
> > fs/gfs2/inode.c | 9 --
> > fs/hfs/inode.c | 2 -
> > fs/hfsplus/dir.c | 3 -
> > fs/hfsplus/inode.c | 3 -
> > fs/jffs2/dir.c | 3 -
> > fs/jffs2/file.c | 3 -
> > fs/jffs2/symlink.c | 3 -
> > fs/jfs/file.c | 3 -
> > fs/jfs/namei.c | 3 -
> > fs/jfs/symlink.c | 6 -
> > fs/kernfs/dir.c | 3 -
> > fs/kernfs/inode.c | 3 -
> > fs/kernfs/symlink.c | 3 -
> > fs/libfs.c | 26 +---
> > fs/nfs/nfs3proc.c | 6 -
> > fs/nfs/nfs4proc.c | 6 -
> > fs/ocfs2/file.c | 3 -
> > fs/ocfs2/namei.c | 3 -
> > fs/ocfs2/symlink.c | 3 -
> > fs/orangefs/inode.c | 3 -
> > fs/orangefs/namei.c | 3 -
> > fs/orangefs/symlink.c | 1 -
> > fs/overlayfs/copy_up.c | 4 -
> > fs/overlayfs/dir.c | 3 -
> > fs/overlayfs/inode.c | 46 +++++--
> > fs/overlayfs/overlayfs.h | 6 +-
> > fs/overlayfs/super.c | 5 +-
> > fs/reiserfs/file.c | 3 -
> > fs/reiserfs/namei.c | 9 --
> > fs/squashfs/inode.c | 1 -
> > fs/squashfs/namei.c | 1 -
> > fs/squashfs/symlink.c | 1 -
> > fs/squashfs/xattr.h | 1 -
> > fs/ubifs/dir.c | 3 -
> > fs/ubifs/file.c | 6 -
> > fs/xattr.c | 231 ++++++++++++++++------------------
> > fs/xfs/xfs_iops.c | 12 --
> > include/linux/fs.h | 5 +-
> > include/linux/xattr.h | 12 +-
> > mm/shmem.c | 15 ---
> > net/socket.c | 1 -
> > security/commoncap.c | 25 ++--
> > security/integrity/evm/evm.h | 7 +-
> > security/integrity/evm/evm_crypto.c | 20 ++-
> > security/integrity/evm/evm_main.c | 5 +-
> > security/integrity/ima/ima_appraise.c | 23 ++--
> > security/selinux/hooks.c | 28 +----
> > security/smack/smack_lsm.c | 28 ++---
> > 71 files changed, 314 insertions(+), 541 deletions(-)
> >
> > --
> > 2.5.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Cheers, Andreas
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH] xattr handlers: fixup generic_listxattr
2016-05-02 23:23 ` [RFC 0/8] Xattr inode operation removal Andreas Dilger
2016-05-03 10:38 ` Andreas Gruenbacher
2016-05-04 20:13 ` James Simmons
@ 2016-05-11 15:54 ` James Simmons
2016-05-11 17:01 ` Andreas Gruenbacher
2 siblings, 1 reply; 23+ messages in thread
From: James Simmons @ 2016-05-11 15:54 UTC (permalink / raw)
To: Alexander Viro
Cc: Andreas Gruenbacher, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, linux-security-module, David Howells,
Serge Hallyn, Dmitry Kasatkin, Paul Moore, Stephen Smalley,
Eric Paris, Casey Schaufler, Oleg Drokin, Andreas Dilger,
Lustre Developement
While looking at porting lustre to use xattr handlers I
noticed issues in generic_listxattr() when handle->list()
is used. The function generic_listxattr() generates it
results from the handle->name field. If the current handle
name field is NULL then generic_listxattr() will call
handle->list() if it exist. Calling handle->list() has no
affect on the output since their is no way to set the
name field of the handler. This patch allows the passing
in of the handler to *list() so the handle->name field
can be set. Now with the ability to create the name field
after its contents are transfer to the passed in buffer
the name field has to be freed to prevent memory leaks.
Also freeing the memory of the name field ensures that
the *list() function will be called at a latter time
which ensures the xattr list is not stale. This patch
is against Viro's work.xattrs branch.
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
fs/ext2/xattr.c | 2 +-
fs/ext4/xattr.c | 2 +-
fs/f2fs/xattr.c | 2 +-
fs/reiserfs/xattr.c | 2 +-
fs/squashfs/xattr.c | 2 +-
fs/xattr.c | 8 ++++++--
include/linux/xattr.h | 2 +-
7 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 1a5e3bf..0aab6a5 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -290,7 +290,7 @@ bad_block: ext2_error(inode->i_sb, "ext2_xattr_list",
const struct xattr_handler *handler =
ext2_xattr_handler(entry->e_name_index);
- if (handler && (!handler->list || handler->list(dentry))) {
+ if (handler && (!handler->list || handler->list(handler, dentry))) {
const char *prefix = handler->prefix ?: handler->name;
size_t prefix_len = strlen(prefix);
size_t size = prefix_len + entry->e_name_len + 1;
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 0441e05..0941198 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -402,7 +402,7 @@ ext4_xattr_list_entries(struct dentry *dentry, struct ext4_xattr_entry *entry,
const struct xattr_handler *handler =
ext4_xattr_handler(entry->e_name_index);
- if (handler && (!handler->list || handler->list(dentry))) {
+ if (handler && (!handler->list || handler->list(handler, dentry))) {
const char *prefix = handler->prefix ?: handler->name;
size_t prefix_len = strlen(prefix);
size_t size = prefix_len + entry->e_name_len + 1;
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 17fd2b1..0cb0fc1 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -412,7 +412,7 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
size_t prefix_len;
size_t size;
- if (!handler || (handler->list && !handler->list(dentry)))
+ if (!handler || (handler->list && !handler->list(handler, dentry)))
continue;
prefix = handler->prefix ?: handler->name;
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 02137bb..509d2a0 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -787,7 +787,7 @@ static int listxattr_filler(struct dir_context *ctx, const char *name,
handler = find_xattr_handler_prefix(b->dentry->d_sb->s_xattr,
name);
if (!handler /* Unsupported xattr name */ ||
- (handler->list && !handler->list(b->dentry)))
+ (handler->list && !handler->list(handler, b->dentry)))
return 0;
size = namelen + 1;
if (b->buf) {
diff --git a/fs/squashfs/xattr.c b/fs/squashfs/xattr.c
index 1548b37..64e8573 100644
--- a/fs/squashfs/xattr.c
+++ b/fs/squashfs/xattr.c
@@ -67,7 +67,7 @@ ssize_t squashfs_listxattr(struct dentry *d, char *buffer,
name_size = le16_to_cpu(entry.size);
handler = squashfs_xattr_handler(le16_to_cpu(entry.type));
- if (handler && (!handler->list || handler->list(d))) {
+ if (handler && (!handler->list || handler->list(handler, d))) {
const char *prefix = handler->prefix ?: handler->name;
size_t prefix_size = strlen(prefix);
diff --git a/fs/xattr.c b/fs/xattr.c
index b11945e..1ec51a7 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -716,9 +716,11 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
if (!buffer) {
for_each_xattr_handler(handlers, handler) {
if (!handler->name ||
- (handler->list && !handler->list(dentry)))
+ (handler->list && !handler->list(handler, dentry)))
continue;
size += strlen(handler->name) + 1;
+ if (handler->list)
+ kfree(handler->name);
}
} else {
char *buf = buffer;
@@ -726,7 +728,7 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
for_each_xattr_handler(handlers, handler) {
if (!handler->name ||
- (handler->list && !handler->list(dentry)))
+ (handler->list && !handler->list(handler, dentry)))
continue;
len = strlen(handler->name);
if (len + 1 > buffer_size)
@@ -734,6 +736,8 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
memcpy(buf, handler->name, len + 1);
buf += len + 1;
buffer_size -= len + 1;
+ if (handler->list)
+ kfree(handler->name);
}
size = buf - buffer;
}
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 1cc4c57..e4fb463 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -28,7 +28,7 @@ struct xattr_handler {
const char *name;
const char *prefix;
int flags; /* fs private flags */
- bool (*list)(struct dentry *dentry);
+ bool (*list)(const struct xattr_handler *, struct dentry *dentry);
int (*get)(const struct xattr_handler *, struct dentry *dentry,
struct inode *inode, const char *name, void *buffer,
size_t size);
--
1.7.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] xattr handlers: fixup generic_listxattr
2016-05-11 15:54 ` [PATCH] xattr handlers: fixup generic_listxattr James Simmons
@ 2016-05-11 17:01 ` Andreas Gruenbacher
2016-05-17 1:12 ` James Simmons
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-11 17:01 UTC (permalink / raw)
To: James Simmons
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, LSM, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler, Oleg Drokin, Andreas Dilger, Lustre Developement
On Wed, May 11, 2016 at 5:54 PM, James Simmons <jsimmons@infradead.org> wrote:
> While looking at porting lustre to use xattr handlers I
> noticed issues in generic_listxattr() when handle->list()
> is used. The function generic_listxattr() generates it
> results from the handle->name field. If the current handle
> name field is NULL then generic_listxattr() will call
> handle->list() if it exist.
generic_listxattr() is different from generic_getxattr() /
generic_setxattr() / generic_removexattr. It makes sense only for
filesystems that support a fixed set of xattrs, which means that all
handlers will have handler->name set.
If any of the handlers has handler->prefix set instead, that handler
matches a whole set of attributes. Generic_listxattr() would have to
fill in all of those names matching that handler, but it doesn't know
which those are.
It is common for filesystems to have their own listxattr inode
operation and still use generic_{get,set,remove}xattr.
> Calling handle->list() has no
> affect on the output since their is no way to set the
> name field of the handler. This patch allows the passing
> in of the handler to *list() so the handle->name field
> can be set.
No, that's broken. The handlers pointed at by sb->s_xattr must not be
modified. Doing so would mess up all other concurrent getxattr /
setxattr / listxattr / removexattr operations on that superblock.
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH] xattr handlers: fixup generic_listxattr
2016-05-11 17:01 ` Andreas Gruenbacher
@ 2016-05-17 1:12 ` James Simmons
2016-05-17 2:03 ` Andreas Gruenbacher
0 siblings, 1 reply; 23+ messages in thread
From: James Simmons @ 2016-05-17 1:12 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, LSM, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler, Oleg Drokin, Andreas Dilger, Lustre Developement
> generic_listxattr() is different from generic_getxattr() /
> generic_setxattr() / generic_removexattr. It makes sense only for
> filesystems that support a fixed set of xattrs, which means that all
> handlers will have handler->name set.
>
> If any of the handlers has handler->prefix set instead, that handler
> matches a whole set of attributes. Generic_listxattr() would have to
> fill in all of those names matching that handler, but it doesn't know
> which those are.
>
> It is common for filesystems to have their own listxattr inode
> operation and still use generic_{get,set,remove}xattr.
That clears things up a bit. So that leaves a few questions. First
question is looking at several of the file system's implementations
I noticed it contains loops such as:
list_for_each_xattr(entry, base_addr) {
const struct xattr_handler *handler =
blah_xattr_handler(entry->e_name_index);
const char *prefix;
size_t prefix_len;
size_t size;
if (!handler || (handler->list && !handler->list(dentry)))
continue;
...
}
Is the handler->list() test needed for a private listxattr implementation?
Also I don't see anyone using handler->list() which which brings up the
next question. What is the purpose of list() function?
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH] xattr handlers: fixup generic_listxattr
2016-05-17 1:12 ` James Simmons
@ 2016-05-17 2:03 ` Andreas Gruenbacher
0 siblings, 0 replies; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-17 2:03 UTC (permalink / raw)
To: James Simmons
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, Mimi Zohar,
linux-ima-devel, LSM, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler, Oleg Drokin, Andreas Dilger, Lustre Developement
On Tue, May 17, 2016 at 3:12 AM, James Simmons <jsimmons@infradead.org> wrote:
>> generic_listxattr() is different from generic_getxattr() /
>> generic_setxattr() / generic_removexattr. It makes sense only for
>> filesystems that support a fixed set of xattrs, which means that all
>> handlers will have handler->name set.
>>
>> If any of the handlers has handler->prefix set instead, that handler
>> matches a whole set of attributes. Generic_listxattr() would have to
>> fill in all of those names matching that handler, but it doesn't know
>> which those are.
>>
>> It is common for filesystems to have their own listxattr inode
>> operation and still use generic_{get,set,remove}xattr.
>
> That clears things up a bit. So that leaves a few questions. First
> question is looking at several of the file system's implementations
> I noticed it contains loops such as:
>
> list_for_each_xattr(entry, base_addr) {
> const struct xattr_handler *handler =
> blah_xattr_handler(entry->e_name_index);
> const char *prefix;
> size_t prefix_len;
> size_t size;
>
> if (!handler || (handler->list && !handler->list(dentry)))
> continue;
>
> ...
> }
>
> Is the handler->list() test needed for a private listxattr implementation?
> Also I don't see anyone using handler->list() which which brings up the
> next question. What is the purpose of list() function?
Trusted xattrs are meant to only be visible to tasks with the
CAP_SYS_ADMIN capability. For deciding which names to include in its
result, listxattr implementations can use the list function, or any
other suitable mechanism. On filesystems that store the complete xattr
names as strings, a strncmp check for a "trusted." prefix together
with capable(CAP_SYS_ADMIN) would do, for example.
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 0/8] Xattr inode operation removal
2016-05-02 22:45 [RFC 0/8] Xattr inode operation removal Andreas Gruenbacher
` (8 preceding siblings ...)
2016-05-02 23:23 ` [RFC 0/8] Xattr inode operation removal Andreas Dilger
@ 2016-05-03 2:40 ` Mimi Zohar
2016-05-03 11:49 ` Andreas Gruenbacher
9 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2016-05-03 2:40 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, fuse-devel, linux-ima-devel,
linux-security-module, David Howells, Serge Hallyn,
Dmitry Kasatkin, Paul Moore, Stephen Smalley, Eric Paris,
Casey Schaufler, Oleg Drokin, Andreas Dilger
Hi Andreas,
On Tue, 2016-05-03 at 00:45 +0200, Andreas Gruenbacher wrote:
> Hi all,
>
> what are your thoughts on this patch set? It applies on top of the
> work.xattr branch [*], converts the remaining filesystems over to xattr
> handlers, and replaces the getxattr, setxattr, and removexattr inode
> operations. The only way to implement getxattr, setxattr, and
> removexattr with this approach is through xattr handlers.
The patch description should provide the motivation/reason for the
change (eg. performance, locking). Up to now these xattr functions
required the caller to take the i_mutex. Is the i_mutex still required?
Mimi
> *** Please don't merge yet: this is boot tested only so far! ***
>
> Lustre currently also breaks; I haven't succeeded in cleaning up the
> mess there. Oleg and Andreas, would you like to look into that?
>
> Thanks,
> Andreas
>
> [*] https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git/log/?h=work.xattr
>
> Andreas Gruenbacher (8):
> ecryptfs: Switch to generic xattr handlers
> overlayfs: Switch to generic xattr handlers
> fuse: Switch to generic xattr handlers
> evm: Turn evm_update_evmxattr into void function
> xattr: Add per-inode xattr handlers as a new inode operation
> xattr: Add __vfs_{get,set,remove}xattr helpers
> xattr: Stop calling {get,set,remove}xattr inode operations
> xattr: Remove generic xattr handlers
>
> fs/9p/vfs_inode_dotl.c | 9 --
> fs/bad_inode.c | 33 +++--
> fs/btrfs/inode.c | 12 --
> fs/cachefiles/bind.c | 4 +-
> fs/cachefiles/namei.c | 4 +-
> fs/ceph/dir.c | 3 -
> fs/ceph/inode.c | 6 -
> fs/cifs/cifsfs.c | 9 --
> fs/ecryptfs/ecryptfs_kernel.h | 2 +
> fs/ecryptfs/inode.c | 64 +++++-----
> fs/ecryptfs/main.c | 1 +
> fs/ecryptfs/mmap.c | 15 +--
> fs/ext2/file.c | 3 -
> fs/ext2/namei.c | 6 -
> fs/ext2/symlink.c | 6 -
> fs/ext4/file.c | 3 -
> fs/ext4/namei.c | 6 -
> fs/ext4/symlink.c | 9 --
> fs/f2fs/file.c | 3 -
> fs/f2fs/namei.c | 12 --
> fs/fuse/dir.c | 40 ++++--
> fs/fuse/fuse_i.h | 2 +
> fs/fuse/inode.c | 1 +
> fs/gfs2/inode.c | 9 --
> fs/hfs/inode.c | 2 -
> fs/hfsplus/dir.c | 3 -
> fs/hfsplus/inode.c | 3 -
> fs/jffs2/dir.c | 3 -
> fs/jffs2/file.c | 3 -
> fs/jffs2/symlink.c | 3 -
> fs/jfs/file.c | 3 -
> fs/jfs/namei.c | 3 -
> fs/jfs/symlink.c | 6 -
> fs/kernfs/dir.c | 3 -
> fs/kernfs/inode.c | 3 -
> fs/kernfs/symlink.c | 3 -
> fs/libfs.c | 26 +---
> fs/nfs/nfs3proc.c | 6 -
> fs/nfs/nfs4proc.c | 6 -
> fs/ocfs2/file.c | 3 -
> fs/ocfs2/namei.c | 3 -
> fs/ocfs2/symlink.c | 3 -
> fs/orangefs/inode.c | 3 -
> fs/orangefs/namei.c | 3 -
> fs/orangefs/symlink.c | 1 -
> fs/overlayfs/copy_up.c | 4 -
> fs/overlayfs/dir.c | 3 -
> fs/overlayfs/inode.c | 46 +++++--
> fs/overlayfs/overlayfs.h | 6 +-
> fs/overlayfs/super.c | 5 +-
> fs/reiserfs/file.c | 3 -
> fs/reiserfs/namei.c | 9 --
> fs/squashfs/inode.c | 1 -
> fs/squashfs/namei.c | 1 -
> fs/squashfs/symlink.c | 1 -
> fs/squashfs/xattr.h | 1 -
> fs/ubifs/dir.c | 3 -
> fs/ubifs/file.c | 6 -
> fs/xattr.c | 231 ++++++++++++++++------------------
> fs/xfs/xfs_iops.c | 12 --
> include/linux/fs.h | 5 +-
> include/linux/xattr.h | 12 +-
> mm/shmem.c | 15 ---
> net/socket.c | 1 -
> security/commoncap.c | 25 ++--
> security/integrity/evm/evm.h | 7 +-
> security/integrity/evm/evm_crypto.c | 20 ++-
> security/integrity/evm/evm_main.c | 5 +-
> security/integrity/ima/ima_appraise.c | 23 ++--
> security/selinux/hooks.c | 28 +----
> security/smack/smack_lsm.c | 28 ++---
> 71 files changed, 314 insertions(+), 541 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 0/8] Xattr inode operation removal
2016-05-03 2:40 ` [RFC 0/8] Xattr inode operation removal Mimi Zohar
@ 2016-05-03 11:49 ` Andreas Gruenbacher
2016-05-03 13:12 ` Mimi Zohar
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Gruenbacher @ 2016-05-03 11:49 UTC (permalink / raw)
To: Mimi Zohar
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, linux-ima-devel, LSM,
David Howells, Serge Hallyn, Dmitry Kasatkin, Paul Moore,
Stephen Smalley, Eric Paris, Casey Schaufler, Oleg Drokin,
Andreas Dilger
On Tue, May 3, 2016 at 4:40 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Hi Andreas,
>
> On Tue, 2016-05-03 at 00:45 +0200, Andreas Gruenbacher wrote:
>> Hi all,
>>
>> what are your thoughts on this patch set? It applies on top of the
>> work.xattr branch [*], converts the remaining filesystems over to xattr
>> handlers, and replaces the getxattr, setxattr, and removexattr inode
>> operations. The only way to implement getxattr, setxattr, and
>> removexattr with this approach is through xattr handlers.
>
> The patch description should provide the motivation/reason for the
> change (eg. performance, locking). Up to now these xattr functions
> required the caller to take the i_mutex. Is the i_mutex still required?
Yes, the documentation needs updating. The locking rules are still the same.
Thanks,
Andreas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 0/8] Xattr inode operation removal
2016-05-03 11:49 ` Andreas Gruenbacher
@ 2016-05-03 13:12 ` Mimi Zohar
0 siblings, 0 replies; 23+ messages in thread
From: Mimi Zohar @ 2016-05-03 13:12 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: Alexander Viro, linux-fsdevel, Tyler Hicks, ecryptfs,
Miklos Szeredi, linux-unionfs, linux-ima-devel, LSM,
David Howells, Serge Hallyn, Dmitry Kasatkin, Paul Moore,
Stephen Smalley, Eric Paris, Casey Schaufler, Oleg Drokin,
Andreas Dilger
On Tue, 2016-05-03 at 13:49 +0200, Andreas Gruenbacher wrote:
> On Tue, May 3, 2016 at 4:40 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Hi Andreas,
> >
> > On Tue, 2016-05-03 at 00:45 +0200, Andreas Gruenbacher wrote:
> >> Hi all,
> >>
> >> what are your thoughts on this patch set? It applies on top of the
> >> work.xattr branch [*], converts the remaining filesystems over to xattr
> >> handlers, and replaces the getxattr, setxattr, and removexattr inode
> >> operations. The only way to implement getxattr, setxattr, and
> >> removexattr with this approach is through xattr handlers.
> >
> > The patch description should provide the motivation/reason for the
> > change (eg. performance, locking). Up to now these xattr functions
> > required the caller to take the i_mutex. Is the i_mutex still required?
>
> Yes, the documentation needs updating. The locking rules are still the same.
I meant to say, this cover letter needs to provide the motivation/reason
for the change(s). From my perspective, the main motivation would
either be "locking" or keeping the file data and metadata in sync. If
these aren't the motivation/reasons for the change, please provide an
explanation.
thanks,
Mimi
^ permalink raw reply [flat|nested] 23+ messages in thread