diff mbox

[3/5] kernfs: Switch to generic xattr handlers

Message ID 1461939517-15497-4-git-send-email-agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Gruenbacher April 29, 2016, 2:18 p.m. UTC 
Unlike with most other filesystems, xattrs in kernfs are attached to the
dentries (dentry->d_fsdata).  Once security modules start calling get
xattr handlers with a NULL dentry (because of an unknown dentry), we'll
have to start check for that in kernfs_xattr_get.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/kernfs/dir.c             |   6 +-
 fs/kernfs/inode.c           | 150 ++++++++++++++++++++++++--------------------
 fs/kernfs/kernfs-internal.h |   6 +-
 fs/kernfs/mount.c           |   1 +
 fs/kernfs/symlink.c         |   6 +-
 5 files changed, 91 insertions(+), 78 deletions(-)

Comments

'Greg Kroah-Hartman' April 30, 2016, 5:02 p.m. UTC | #1 
On Fri, Apr 29, 2016 at 04:18:35PM +0200, Andreas Gruenbacher wrote:
> Unlike with most other filesystems, xattrs in kernfs are attached to the
> dentries (dentry->d_fsdata).  Once security modules start calling get
> xattr handlers with a NULL dentry (because of an unknown dentry), we'll
> have to start check for that in kernfs_xattr_get.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Note, I'm not the only maintainer of kernfs...

Anyway:
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

--
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
Al Viro April 30, 2016, 5:25 p.m. UTC | #2 
On Sat, Apr 30, 2016 at 10:02:31AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 29, 2016 at 04:18:35PM +0200, Andreas Gruenbacher wrote:
> > Unlike with most other filesystems, xattrs in kernfs are attached to the
> > dentries (dentry->d_fsdata).  Once security modules start calling get
> > xattr handlers with a NULL dentry (because of an unknown dentry), we'll
> > have to start check for that in kernfs_xattr_get.
> > 
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Note, I'm not the only maintainer of kernfs...
> 
> Anyway:
> 	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Guys, kernfs uses ->d_fsdata to get to them, but note
static void kernfs_init_inode(struct kernfs_node *kn, struct inode *inode)
{
        kernfs_get(kn);
        inode->i_private = kn;
IOW, inode->i_private would work just as well - it points to the same thing.
--
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
Andreas Grünbacher May 1, 2016, 10:45 a.m. UTC | #3 
2016-04-30 19:02 GMT+02:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Fri, Apr 29, 2016 at 04:18:35PM +0200, Andreas Gruenbacher wrote:
>> Unlike with most other filesystems, xattrs in kernfs are attached to the
>> dentries (dentry->d_fsdata).  Once security modules start calling get
>> xattr handlers with a NULL dentry (because of an unknown dentry), we'll
>> have to start check for that in kernfs_xattr_get.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Note, I'm not the only maintainer of kernfs...

Hmm, that isn't clear from the MAINTAINERS file:

DRIVER CORE, KOBJECTS, DEBUGFS, KERNFS AND SYSFS
M:      Greg Kroah-Hartman <gregkh@linuxfoundation.org>
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
S:      Supported
F:      Documentation/kobject.txt
F:      drivers/base/
F:      fs/debugfs/
F:      fs/kernfs/
F:      fs/sysfs/
F:      include/linux/debugfs.h
F:      include/linux/kobj*
F:      lib/kobj*

> Anyway:
>         Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks,
Andreas
--
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
'Greg Kroah-Hartman' May 1, 2016, 4:27 p.m. UTC | #4 
On Sun, May 01, 2016 at 12:45:53PM +0200, Andreas Grünbacher wrote:
> 2016-04-30 19:02 GMT+02:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> > On Fri, Apr 29, 2016 at 04:18:35PM +0200, Andreas Gruenbacher wrote:
> >> Unlike with most other filesystems, xattrs in kernfs are attached to the
> >> dentries (dentry->d_fsdata).  Once security modules start calling get
> >> xattr handlers with a NULL dentry (because of an unknown dentry), we'll
> >> have to start check for that in kernfs_xattr_get.
> >>
> >> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > Note, I'm not the only maintainer of kernfs...
> 
> Hmm, that isn't clear from the MAINTAINERS file:
> 
> DRIVER CORE, KOBJECTS, DEBUGFS, KERNFS AND SYSFS
> M:      Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> T:      git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
> S:      Supported
> F:      Documentation/kobject.txt
> F:      drivers/base/
> F:      fs/debugfs/
> F:      fs/kernfs/
> F:      fs/sysfs/
> F:      include/linux/debugfs.h
> F:      include/linux/kobj*
> F:      lib/kobj*

Ah, sneaky Tejun, he didn't put his name on it, but if you use
get_maintainer.pl he should show up :)

thanks,

greg k-h

--
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
diff mbox

Patch

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 03b688d..f67d896 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1125,9 +1125,9 @@  const struct inode_operations kernfs_dir_iops = {
 	.permission	= kernfs_iop_permission,
 	.setattr	= kernfs_iop_setattr,
 	.getattr	= kernfs_iop_getattr,
-	.setxattr	= kernfs_iop_setxattr,
-	.removexattr	= kernfs_iop_removexattr,
-	.getxattr	= kernfs_iop_getxattr,
+	.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 b524722..1dfb44a 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -28,9 +28,9 @@  static const struct inode_operations kernfs_iops = {
 	.permission	= kernfs_iop_permission,
 	.setattr	= kernfs_iop_setattr,
 	.getattr	= kernfs_iop_getattr,
-	.setxattr	= kernfs_iop_setxattr,
-	.removexattr	= kernfs_iop_removexattr,
-	.getxattr	= kernfs_iop_getxattr,
+	.setxattr	= generic_setxattr,
+	.removexattr	= generic_removexattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= kernfs_iop_listxattr,
 };
 
@@ -157,70 +157,6 @@  static int kernfs_node_setsecdata(struct kernfs_node *kn, void **secdata,
 	return 0;
 }
 
-int kernfs_iop_setxattr(struct dentry *dentry, const char *name,
-			const void *value, size_t size, int flags)
-{
-	struct kernfs_node *kn = dentry->d_fsdata;
-	struct kernfs_iattrs *attrs;
-	void *secdata;
-	int error;
-	u32 secdata_len = 0;
-
-	attrs = kernfs_iattrs(kn);
-	if (!attrs)
-		return -ENOMEM;
-
-	if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
-		const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
-		error = security_inode_setsecurity(d_inode(dentry), suffix,
-						value, size, flags);
-		if (error)
-			return error;
-		error = security_inode_getsecctx(d_inode(dentry),
-						&secdata, &secdata_len);
-		if (error)
-			return error;
-
-		mutex_lock(&kernfs_mutex);
-		error = kernfs_node_setsecdata(kn, &secdata, &secdata_len);
-		mutex_unlock(&kernfs_mutex);
-
-		if (secdata)
-			security_release_secctx(secdata, secdata_len);
-		return error;
-	} else if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN)) {
-		return simple_xattr_set(&attrs->xattrs, name, value, size,
-					flags);
-	}
-
-	return -EINVAL;
-}
-
-int kernfs_iop_removexattr(struct dentry *dentry, const char *name)
-{
-	struct kernfs_node *kn = dentry->d_fsdata;
-	struct kernfs_iattrs *attrs;
-
-	attrs = kernfs_iattrs(kn);
-	if (!attrs)
-		return -ENOMEM;
-
-	return simple_xattr_set(&attrs->xattrs, name, NULL, 0, XATTR_REPLACE);
-}
-
-ssize_t kernfs_iop_getxattr(struct dentry *unused, struct inode *inode,
-			    const char *name, void *buf, size_t size)
-{
-	struct kernfs_node *kn = inode->i_private;
-	struct kernfs_iattrs *attrs;
-
-	attrs = kernfs_iattrs(kn);
-	if (!attrs)
-		return -ENOMEM;
-
-	return simple_xattr_get(&attrs->xattrs, name, buf, size);
-}
-
 ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
 {
 	struct kernfs_node *kn = dentry->d_fsdata;
@@ -370,3 +306,83 @@  int kernfs_iop_permission(struct inode *inode, int mask)
 
 	return generic_permission(inode, mask);
 }
+
+static int kernfs_xattr_get(const struct xattr_handler *handler,
+			    struct dentry *dentry, struct inode *inode,
+			    const char *suffix, void *value, size_t size)
+{
+	const char *name = xattr_full_name(handler, suffix);
+	struct kernfs_node *kn;
+	struct kernfs_iattrs *attrs;
+
+	kn = dentry->d_fsdata;
+	attrs = kernfs_iattrs(kn);
+	if (!attrs)
+		return -ENOMEM;
+
+	return simple_xattr_get(&attrs->xattrs, name, value, size);
+}
+
+static int kernfs_xattr_set(const struct xattr_handler *handler,
+			    struct dentry *dentry, const char *suffix,
+			    const void *value, size_t size, int flags)
+{
+	const char *name = xattr_full_name(handler, suffix);
+	struct kernfs_node *kn = dentry->d_fsdata;
+	struct kernfs_iattrs *attrs;
+
+	attrs = kernfs_iattrs(kn);
+	if (!attrs)
+		return -ENOMEM;
+
+	return simple_xattr_set(&attrs->xattrs, name, value, size, flags);
+}
+
+const struct xattr_handler kernfs_trusted_xattr_handler = {
+	.prefix = XATTR_TRUSTED_PREFIX,
+	.get = kernfs_xattr_get,
+	.set = kernfs_xattr_set,
+};
+
+static int kernfs_security_xattr_set(const struct xattr_handler *handler,
+				     struct dentry *dentry, const char *suffix,
+				     const void *value, size_t size, int flags)
+{
+	struct kernfs_node *kn = dentry->d_fsdata;
+	struct inode *inode = d_inode(dentry);
+	struct kernfs_iattrs *attrs;
+	void *secdata;
+	u32 secdata_len = 0;
+	int error;
+
+	attrs = kernfs_iattrs(kn);
+	if (!attrs)
+		return -ENOMEM;
+
+	error = security_inode_setsecurity(inode, suffix, value, size, flags);
+	if (error)
+		return error;
+	error = security_inode_getsecctx(inode, &secdata, &secdata_len);
+	if (error)
+		return error;
+
+	mutex_lock(&kernfs_mutex);
+	error = kernfs_node_setsecdata(kn, &secdata, &secdata_len);
+	mutex_unlock(&kernfs_mutex);
+
+	if (secdata)
+		security_release_secctx(secdata, secdata_len);
+	return error;
+}
+
+const struct xattr_handler kernfs_security_xattr_handler = {
+	.prefix = XATTR_SECURITY_PREFIX,
+	.get = kernfs_xattr_get,
+	.set = kernfs_security_xattr_set,
+};
+
+const struct xattr_handler *kernfs_xattr_handlers[] = {
+	&kernfs_trusted_xattr_handler,
+	&kernfs_security_xattr_handler,
+	NULL
+};
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 45c9192..bfd551b 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -76,16 +76,12 @@  extern struct kmem_cache *kernfs_node_cache;
 /*
  * inode.c
  */
+extern const struct xattr_handler *kernfs_xattr_handlers[];
 void kernfs_evict_inode(struct inode *inode);
 int kernfs_iop_permission(struct inode *inode, int mask);
 int kernfs_iop_setattr(struct dentry *dentry, struct iattr *iattr);
 int kernfs_iop_getattr(struct vfsmount *mnt, struct dentry *dentry,
 		       struct kstat *stat);
-int kernfs_iop_setxattr(struct dentry *dentry, const char *name, const void *value,
-			size_t size, int flags);
-int kernfs_iop_removexattr(struct dentry *dentry, const char *name);
-ssize_t kernfs_iop_getxattr(struct dentry *dentry, struct inode *inode,
-			    const char *name, void *buf, size_t size);
 ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
 
 /*
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index b67dbcc..5446c3b 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -142,6 +142,7 @@  static int kernfs_fill_super(struct super_block *sb, unsigned long magic)
 	sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
 	sb->s_magic = magic;
 	sb->s_op = &kernfs_sops;
+	sb->s_xattr = kernfs_xattr_handlers;
 	sb->s_time_gran = 1;
 
 	/* get root inode, initialize and unlock it */
diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c
index 117b8b3..549a14c7 100644
--- a/fs/kernfs/symlink.c
+++ b/fs/kernfs/symlink.c
@@ -134,9 +134,9 @@  static const char *kernfs_iop_get_link(struct dentry *dentry,
 }
 
 const struct inode_operations kernfs_symlink_iops = {
-	.setxattr	= kernfs_iop_setxattr,
-	.removexattr	= kernfs_iop_removexattr,
-	.getxattr	= kernfs_iop_getxattr,
+	.setxattr	= generic_setxattr,
+	.removexattr	= generic_removexattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= kernfs_iop_listxattr,
 	.readlink	= generic_readlink,
 	.get_link	= kernfs_iop_get_link,