Messages in this thread | | | Date | Thu, 2 Dec 2021 08:47:53 -0500 | Subject | Re: [RFC 13/20] securityfs: Build securityfs_ns for namespacing support | From | Stefan Berger <> |
| |
On 12/2/21 08:35, Christian Brauner wrote: > On Tue, Nov 30, 2021 at 11:06:47AM -0500, Stefan Berger wrote: >> Implement 'securityfs_ns' for support of IMA namespacing so that each >> IMA (user) namespace can have its own front-end for showing the currently >> active policy, the measurement list, number of violations and so on. This >> filesystem shares much of the existing code of SecurityFS but requires a >> new API call securityfs_ns_create_mount() for creating a new instance. >> >> The API calls of securityfs_ns have the prefix securityfs_ns_ and take >> additional parameters struct vfsmount * and mount_count that allow for >> multiple instances of this filesystem to exist. >> >> The filesystem can be mounted to the usual securityfs mount point like >> this: >> >> mount -t securityfs_ns /sys/kernel/security /sys/kernel/security >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> include/linux/security.h | 18 ++++ >> include/uapi/linux/magic.h | 1 + >> security/inode.c | 197 +++++++++++++++++++++++++++++++++++-- >> 3 files changed, 210 insertions(+), 6 deletions(-) >> >> diff --git a/include/linux/security.h b/include/linux/security.h >> index 7e0ba63b5dde..8e479266f544 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -1929,6 +1929,24 @@ struct dentry *securityfs_create_symlink(const char *name, >> const struct inode_operations *iops); >> extern void securityfs_remove(struct dentry *dentry); >> >> +extern struct dentry *securityfs_ns_create_file(const char *name, umode_t mode, >> + struct dentry *parent, void *data, >> + const struct file_operations *fops, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count); >> +extern struct dentry *securityfs_ns_create_dir(const char *name, struct dentry *parent, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count); >> +struct dentry *securityfs_ns_create_symlink(const char *name, >> + struct dentry *parent, >> + const char *target, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count); >> +extern void securityfs_ns_remove(struct dentry *dentry, >> + struct vfsmount **mount, int *mount_count); >> +struct vfsmount *securityfs_ns_create_mount(struct user_namespace *user_ns); >> +extern struct vfsmount *securityfs_ns_mount; >> + >> #else /* CONFIG_SECURITYFS */ >> >> static inline struct dentry *securityfs_create_dir(const char *name, >> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h >> index 35687dcb1a42..5c1cc6088dd2 100644 >> --- a/include/uapi/linux/magic.h >> +++ b/include/uapi/linux/magic.h >> @@ -11,6 +11,7 @@ >> #define CRAMFS_MAGIC_WEND 0x453dcd28 /* magic number with the wrong endianess */ >> #define DEBUGFS_MAGIC 0x64626720 >> #define SECURITYFS_MAGIC 0x73636673 >> +#define SECURITYFS_NS_MAGIC 0x73334473 >> #define SELINUX_MAGIC 0xf97cff8c >> #define SMACK_MAGIC 0x43415d53 /* "SMAC" */ >> #define RAMFS_MAGIC 0x858458f6 /* some random number */ >> diff --git a/security/inode.c b/security/inode.c >> index 429744ff4ab3..8077d1f31489 100644 >> --- a/security/inode.c >> +++ b/security/inode.c >> @@ -21,6 +21,7 @@ >> #include <linux/security.h> >> #include <linux/lsm_hooks.h> >> #include <linux/magic.h> >> +#include <linux/user_namespace.h> >> >> static struct vfsmount *securityfs_mount; >> static int securityfs_mount_count; >> @@ -73,6 +74,61 @@ static struct file_system_type securityfs_type = { >> .kill_sb = kill_litter_super, >> }; >> >> +static int securityfs_ns_fill_super(struct super_block *sb, struct fs_context *fc) >> +{ >> + static const struct tree_descr files[] = {{""}}; >> + int error; >> + >> + error = simple_fill_super(sb, SECURITYFS_NS_MAGIC, files); >> + if (error) >> + return error; >> + >> + sb->s_op = &securityfs_super_operations; >> + >> + return 0; >> +} >> + >> +static int securityfs_ns_get_tree(struct fs_context *fc) >> +{ >> + return get_tree_keyed(fc, securityfs_ns_fill_super, fc->user_ns); >> +} >> + >> +static const struct fs_context_operations securityfs_ns_context_ops = { >> + .get_tree = securityfs_ns_get_tree, >> +}; >> + >> +static int securityfs_ns_init_fs_context(struct fs_context *fc) >> +{ >> + fc->ops = &securityfs_ns_context_ops; >> + return 0; >> +} >> + >> +static struct file_system_type securityfs_ns_type = { >> + .owner = THIS_MODULE, >> + .name = "securityfs_ns", >> + .init_fs_context = securityfs_ns_init_fs_context, >> + .kill_sb = kill_litter_super, >> + .fs_flags = FS_USERNS_MOUNT, >> +}; >> + >> +struct vfsmount *securityfs_ns_create_mount(struct user_namespace *user_ns) >> +{ >> + struct fs_context *fc; >> + struct vfsmount *mnt; >> + >> + fc = fs_context_for_mount(&securityfs_ns_type, SB_KERNMOUNT); >> + if (IS_ERR(fc)) >> + return ERR_CAST(fc); >> + >> + put_user_ns(fc->user_ns); >> + fc->user_ns = get_user_ns(user_ns); >> + >> + mnt = fc_mount(fc); >> + put_fs_context(fc); >> + return mnt; >> +} >> + >> + >> /** >> * securityfs_create_dentry - create a dentry in the securityfs filesystem >> * >> @@ -155,8 +211,8 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, >> inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode); >> inode->i_private = data; >> if (S_ISDIR(mode)) { >> - inode->i_op = &simple_dir_inode_operations; >> - inode->i_fop = &simple_dir_operations; >> + inode->i_op = iops ? iops : &simple_dir_inode_operations; >> + inode->i_fop = fops ? fops : &simple_dir_operations; >> inc_nlink(inode); >> inc_nlink(dir); >> } else if (S_ISLNK(mode)) { >> @@ -214,6 +270,41 @@ struct dentry *securityfs_create_file(const char *name, umode_t mode, >> } >> EXPORT_SYMBOL_GPL(securityfs_create_file); >> >> +/** >> + * securityfs_ns_create_file - create a file in the securityfs_ns filesystem >> + * >> + * @name: a pointer to a string containing the name of the file to create. >> + * @mode: the permission that the file should have >> + * @parent: a pointer to the parent dentry for this file. This should be a >> + * directory dentry if set. If this parameter is %NULL, then the >> + * file will be created in the root of the securityfs_ns filesystem. >> + * @data: a pointer to something that the caller will want to get to later >> + * on. The inode.i_private pointer will point to this value on >> + * the open() call. >> + * @fops: a pointer to a struct file_operations that should be used for >> + * this file. >> + * @mount: Pointer to a pointer of a an existing vfsmount >> + * @mount_count: The mount_count that goes along with the @mount >> + * >> + * This function creates a file in securityfs_ns with the given @name. >> + * >> + * This function returns a pointer to a dentry if it succeeds. This >> + * pointer must be passed to the securityfs_ns_remove() function when the file >> + * is to be removed (no automatic cleanup happens if your module is unloaded, >> + * you are responsible here). If an error occurs, the function will return >> + * the error value (via ERR_PTR). >> + */ >> +struct dentry *securityfs_ns_create_file(const char *name, umode_t mode, >> + struct dentry *parent, void *data, >> + const struct file_operations *fops, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count) >> +{ >> + return securityfs_create_dentry(name, mode, parent, data, fops, iops, >> + &securityfs_ns_type, mount, mount_count); >> +} >> +EXPORT_SYMBOL_GPL(securityfs_ns_create_file); >> + >> /** >> * securityfs_create_dir - create a directory in the securityfs filesystem >> * >> @@ -240,6 +331,34 @@ struct dentry *securityfs_create_dir(const char *name, struct dentry *parent) >> } >> EXPORT_SYMBOL_GPL(securityfs_create_dir); >> >> +/** >> + * securityfs_ns_create_dir - create a directory in the securityfs_ns filesystem >> + * >> + * @name: a pointer to a string containing the name of the directory to >> + * create. >> + * @parent: a pointer to the parent dentry for this file. This should be a >> + * directory dentry if set. If this parameter is %NULL, then the >> + * directory will be created in the root of the securityfs_ns filesystem. >> + * @mount: Pointer to a pointer of a an existing vfsmount >> + * @mount_count: The mount_count that goes along with the @mount >> + * >> + * This function creates a directory in securityfs_ns with the given @name. >> + * >> + * This function returns a pointer to a dentry if it succeeds. This >> + * pointer must be passed to the securityfs_ns_remove() function when the file >> + * is to be removed (no automatic cleanup happens if your module is unloaded, >> + * you are responsible here). If an error occurs, the function will return >> + * the error value (via ERR_PTR). >> + */ >> +struct dentry *securityfs_ns_create_dir(const char *name, struct dentry *parent, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count) >> +{ >> + return securityfs_ns_create_file(name, S_IFDIR | 0755, parent, NULL, NULL, >> + iops, mount, mount_count); >> +} >> +EXPORT_SYMBOL_GPL(securityfs_ns_create_dir); >> + >> struct dentry *_securityfs_create_symlink(const char *name, >> struct dentry *parent, >> const char *target, >> @@ -263,6 +382,7 @@ struct dentry *_securityfs_create_symlink(const char *name, >> >> return dent; >> } >> + >> /** >> * securityfs_create_symlink - create a symlink in the securityfs filesystem >> * >> @@ -300,6 +420,42 @@ struct dentry *securityfs_create_symlink(const char *name, >> } >> EXPORT_SYMBOL_GPL(securityfs_create_symlink); >> >> +/** >> + * securityfs_ns_create_symlink - create a symlink in the securityfs_ns filesystem >> + * >> + * @name: a pointer to a string containing the name of the symlink to >> + * create. >> + * @parent: a pointer to the parent dentry for the symlink. This should be a >> + * directory dentry if set. If this parameter is %NULL, then the >> + * directory will be created in the root of the securityfs_ns filesystem. >> + * @target: a pointer to a string containing the name of the symlink's target. >> + * If this parameter is %NULL, then the @iops parameter needs to be >> + * setup to handle .readlink and .get_link inode_operations. >> + * @iops: a pointer to the struct inode_operations to use for the symlink. If >> + * this parameter is %NULL, then the default simple_symlink_inode >> + * operations will be used. >> + * @mount: Pointer to a pointer of a an existing vfsmount >> + * @mount_count: The mount_count that goes along with the @mount >> + * >> + * This function creates a symlink in securityfs_ns with the given @name. >> + * >> + * This function returns a pointer to a dentry if it succeeds. This >> + * pointer must be passed to the securityfs_ns_remove() function when the file >> + * is to be removed (no automatic cleanup happens if your module is unloaded, >> + * you are responsible here). If an error occurs, the function will return >> + * the error value (via ERR_PTR). >> + */ >> +struct dentry *securityfs_ns_create_symlink(const char *name, >> + struct dentry *parent, >> + const char *target, >> + const struct inode_operations *iops, >> + struct vfsmount **mount, int *mount_count) >> +{ >> + return _securityfs_create_symlink(name, parent, target, iops, >> + &securityfs_ns_type, mount, mount_count); >> +} >> +EXPORT_SYMBOL_GPL(securityfs_ns_create_symlink); >> + >> void _securityfs_remove(struct dentry *dentry, struct vfsmount **mount, int *mount_count) >> { >> struct inode *dir; >> @@ -340,6 +496,27 @@ void securityfs_remove(struct dentry *dentry) >> >> EXPORT_SYMBOL_GPL(securityfs_remove); >> >> +/** >> + * securityfs_ns_remove - removes a file or directory from the securityfs_ns filesystem >> + * >> + * @dentry: a pointer to a the dentry of the file or directory to be removed. >> + * @mount: Pointer to a pointer of a an existing vfsmount >> + * @mount_count: The mount_count that goes along with the @mount >> + * >> + * This function removes a file or directory in securityfs_ns that was previously >> + * created with a call to another securityfs_ns function (like >> + * securityfs_ns_create_file() or variants thereof.) >> + * >> + * This function is required to be called in order for the file to be >> + * removed. No automatic cleanup of files will happen when a module is >> + * removed; you are responsible here. >> + */ >> +void securityfs_ns_remove(struct dentry *dentry, struct vfsmount **mount, int *mount_count) >> +{ >> + _securityfs_remove(dentry, mount, mount_count); >> +} >> +EXPORT_SYMBOL_GPL(securityfs_ns_remove); >> + >> #ifdef CONFIG_SECURITY >> static struct dentry *lsm_dentry; >> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, >> @@ -364,14 +541,22 @@ static int __init securityfs_init(void) >> return retval; >> >> retval = register_filesystem(&securityfs_type); >> - if (retval) { >> - sysfs_remove_mount_point(kernel_kobj, "security"); >> - return retval; >> - } >> + if (retval) >> + goto remove_mount; >> + retval = register_filesystem(&securityfs_ns_type); >> + if (retval) >> + goto unregister_filesystem; > So you're introducing a new filesystem type securityfs_ns. Ithink that's > simply wrong and feels like a hack. What issues did you run into when > trying to convert the existing securityfs itself?
I primarily didn't want to touch the existing securityfs with its existing users and it being a single instance filesystem. So I though I'd create something with a new API just for namespaces that is multi-instance capable.
> > I see no immediate reason why a get_tree_keyed() conversion for > securityfs wouldn't work even with the debugfs pin/unpin logic in there > kept for the securityfs mounted in the initial userns. Ok, let me try to convert securityfs then.
| |