* [Virtio-fs] [PATCH] RFC: fuse: virtiofs: Call security hooks on new inodes
@ 2020-06-01 5:32 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-01 5:32 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Suleiman Souhlal, virtio-fs, linux-fsdevel, Dylan Reid, Vivek Goyal
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Set this to true for
virtiofs but false for regular fuse file systems.
Calling security hooks is needed for `setfscreatecon` to work since it
is applied as part of the selinux security hook.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
fs/fuse/dir.c | 74 ++++++++++++++++++++++++++++++++++++++++++---
fs/fuse/fuse_i.h | 4 +++
fs/fuse/inode.c | 1 +
fs/fuse/virtio_fs.c | 1 +
4 files changed, 75 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index de1e2fde60bd4..b18c92a8a4c11 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -135,6 +138,50 @@ static void fuse_dir_changed(struct inode *dir)
inode_maybe_inc_iversion(dir, false);
}
+static int fuse_initxattrs(struct inode *inode, const struct xattr *xattrs,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ int err = 0;
+ int len;
+ char *name;
+
+ for (xattr = xattrs; xattr->name != NULL; ++xattr) {
+ len = XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name) + 1;
+ name = kmalloc(len, GFP_KERNEL);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+
+ scnprintf(name, len, XATTR_SECURITY_PREFIX "%s", xattr->name);
+ err = fuse_setxattr(inode, name, xattr->value, xattr->value_len,
+ 0);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+
+ return err;
+}
+
+/*
+ * Initialize security on newly created inodes if supported by the filesystem.
+ */
+static int fuse_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ struct fuse_conn *conn = get_fuse_conn(dir);
+ int err = 0;
+
+ if (conn->init_security) {
+ err = security_inode_init_security(inode, dir, qstr,
+ fuse_initxattrs, NULL);
+ }
+
+ return err;
+}
+
/**
* Mark the attributes as stale due to an atime change. Avoid the invalidate if
* atime is not used.
@@ -498,7 +545,17 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
err = -ENOMEM;
goto out_err;
}
+
+ err = fuse_init_security(inode, dir, &entry->d_name);
+ if (err) {
+ flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
+ fi = get_fuse_inode(inode);
+ fuse_sync_release(fi, ff, flags);
+ fuse_queue_forget(fc, forget, outentry.nodeid, 1);
+ goto out_err;
+ }
kfree(forget);
+
d_instantiate(entry, inode);
fuse_change_entry_timeout(entry, &outentry);
fuse_dir_changed(dir);
@@ -569,7 +626,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
*/
static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
struct inode *dir, struct dentry *entry,
- umode_t mode)
+ umode_t mode, bool init_security)
{
struct fuse_entry_out outarg;
struct inode *inode;
@@ -603,6 +660,13 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
fuse_queue_forget(fc, forget, outarg.nodeid, 1);
return -ENOMEM;
}
+ if (init_security) {
+ err = fuse_init_security(inode, dir, &entry->d_name);
+ if (err) {
+ fuse_queue_forget(fc, forget, outarg.nodeid, 1);
+ return err;
+ }
+ }
kfree(forget);
d_drop(entry);
@@ -644,7 +708,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+ return create_new_entry(fc, &args, dir, entry, mode, true);
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -671,7 +735,7 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+ return create_new_entry(fc, &args, dir, entry, S_IFDIR, true);
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -687,7 +751,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+ return create_new_entry(fc, &args, dir, entry, S_IFLNK, true);
}
void fuse_update_ctime(struct inode *inode)
@@ -858,7 +922,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
args.in_args[0].value = &inarg;
args.in_args[1].size = newent->d_name.len + 1;
args.in_args[1].value = newent->d_name.name;
- err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
+ err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
/* Contrary to "normal" filesystems it can happen that link
makes two "logical" inodes point to the same "physical"
inode. We invalidate the attributes of the old one, so it
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ca344bf714045..ed871742db584 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -482,6 +482,7 @@ struct fuse_fs_context {
bool no_control:1;
bool no_force_umount:1;
bool no_mount_options:1;
+ bool init_security:1;
unsigned int max_read;
unsigned int blksize;
const char *subtype;
@@ -719,6 +720,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security : 1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 95d712d44ca13..ab47e73566864 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1179,6 +1179,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
fc->no_control = ctx->no_control;
fc->no_force_umount = ctx->no_force_umount;
fc->no_mount_options = ctx->no_mount_options;
+ fc->init_security = ctx->init_security;
err = -ENOMEM;
root = fuse_get_root_inode(sb, ctx->rootmode);
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index bade747689033..ee22e9a8309df 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1051,6 +1051,7 @@ static int virtio_fs_fill_super(struct super_block *sb)
.no_control = true,
.no_force_umount = true,
.no_mount_options = true,
+ .init_security = true,
};
mutex_lock(&virtio_fs_mutex);
--
2.27.0.rc0.183.gde8f92d652-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH] RFC: fuse: virtiofs: Call security hooks on new inodes
2020-06-01 5:32 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-06-02 18:23 ` Vivek Goyal
-1 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-06-02 18:23 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Miklos Szeredi, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, linux-security-module, selinux
On Mon, Jun 01, 2020 at 02:32:14PM +0900, Chirantan Ekbote wrote:
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Set this to true for
> virtiofs but false for regular fuse file systems.
>
> Calling security hooks is needed for `setfscreatecon` to work since it
> is applied as part of the selinux security hook.
>
> Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
> ---
> fs/fuse/dir.c | 74 ++++++++++++++++++++++++++++++++++++++++++---
> fs/fuse/fuse_i.h | 4 +++
> fs/fuse/inode.c | 1 +
> fs/fuse/virtio_fs.c | 1 +
> 4 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index de1e2fde60bd4..b18c92a8a4c11 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -16,6 +16,9 @@
> #include <linux/xattr.h>
> #include <linux/iversion.h>
> #include <linux/posix_acl.h>
> +#include <linux/security.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
>
> static void fuse_advise_use_readdirplus(struct inode *dir)
> {
> @@ -135,6 +138,50 @@ static void fuse_dir_changed(struct inode *dir)
> inode_maybe_inc_iversion(dir, false);
> }
>
> +static int fuse_initxattrs(struct inode *inode, const struct xattr *xattrs,
> + void *fs_info)
> +{
> + const struct xattr *xattr;
> + int err = 0;
> + int len;
> + char *name;
> +
> + for (xattr = xattrs; xattr->name != NULL; ++xattr) {
> + len = XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name) + 1;
> + name = kmalloc(len, GFP_KERNEL);
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> +
> + scnprintf(name, len, XATTR_SECURITY_PREFIX "%s", xattr->name);
> + err = fuse_setxattr(inode, name, xattr->value, xattr->value_len,
> + 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> +
> + return err;
> +}
> +
> +/*
> + * Initialize security on newly created inodes if supported by the filesystem.
> + */
> +static int fuse_init_security(struct inode *inode, struct inode *dir,
> + const struct qstr *qstr)
> +{
> + struct fuse_conn *conn = get_fuse_conn(dir);
> + int err = 0;
> +
> + if (conn->init_security) {
> + err = security_inode_init_security(inode, dir, qstr,
> + fuse_initxattrs, NULL);
> + }
> +
> + return err;
> +}
> +
> /**
> * Mark the attributes as stale due to an atime change. Avoid the invalidate if
> * atime is not used.
> @@ -498,7 +545,17 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> err = -ENOMEM;
> goto out_err;
> }
> +
> + err = fuse_init_security(inode, dir, &entry->d_name);
> + if (err) {
> + flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> + fi = get_fuse_inode(inode);
> + fuse_sync_release(fi, ff, flags);
> + fuse_queue_forget(fc, forget, outentry.nodeid, 1);
> + goto out_err;
> + }
> kfree(forget);
> +
[ cc lsm and selinux list ]
So this sets xattr after file creation. But this is not atomic w.r.t
file creation. I think keeping file creation and selinux context setting
to be atomic was one of the requirements.
Can we first retrieve the label which will be created for inode
(using dentry perhaps) and then pass that label as part of CREATE/MKNOD
request and then server can set fscreate (per thread) before file
creation. I hope /proc/[pid]/attr/fscreate work for per thread too.
Stephen had mentioned dentry_init_security() for this. Overlayfs uses
a variant of the same hook dentry_create_files_as().
> d_instantiate(entry, inode);
> fuse_change_entry_timeout(entry, &outentry);
> fuse_dir_changed(dir);
> @@ -569,7 +626,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
> */
> static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
> struct inode *dir, struct dentry *entry,
> - umode_t mode)
> + umode_t mode, bool init_security)
> {
> struct fuse_entry_out outarg;
> struct inode *inode;
> @@ -603,6 +660,13 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
> fuse_queue_forget(fc, forget, outarg.nodeid, 1);
> return -ENOMEM;
> }
> + if (init_security) {
> + err = fuse_init_security(inode, dir, &entry->d_name);
> + if (err) {
> + fuse_queue_forget(fc, forget, outarg.nodeid, 1);
> + return err;
> + }
> + }
> kfree(forget);
>
> d_drop(entry);
> @@ -644,7 +708,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, mode);
> + return create_new_entry(fc, &args, dir, entry, mode, true);
> }
>
> static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
> @@ -671,7 +735,7 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, S_IFDIR);
> + return create_new_entry(fc, &args, dir, entry, S_IFDIR, true);
> }
>
> static int fuse_symlink(struct inode *dir, struct dentry *entry,
> @@ -687,7 +751,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> args.in_args[0].value = entry->d_name.name;
> args.in_args[1].size = len;
> args.in_args[1].value = link;
> - return create_new_entry(fc, &args, dir, entry, S_IFLNK);
> + return create_new_entry(fc, &args, dir, entry, S_IFLNK, true);
> }
>
> void fuse_update_ctime(struct inode *inode)
> @@ -858,7 +922,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = newent->d_name.len + 1;
> args.in_args[1].value = newent->d_name.name;
> - err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
> + err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
> /* Contrary to "normal" filesystems it can happen that link
> makes two "logical" inodes point to the same "physical"
> inode. We invalidate the attributes of the old one, so it
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ca344bf714045..ed871742db584 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -482,6 +482,7 @@ struct fuse_fs_context {
> bool no_control:1;
> bool no_force_umount:1;
> bool no_mount_options:1;
> + bool init_security:1;
> unsigned int max_read;
> unsigned int blksize;
> const char *subtype;
> @@ -719,6 +720,9 @@ struct fuse_conn {
> /* Do not show mount options */
> unsigned int no_mount_options:1;
>
> + /* Initialize security xattrs when creating a new inode */
> + unsigned int init_security : 1;
> +
> /** The number of requests waiting for completion */
> atomic_t num_waiting;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 95d712d44ca13..ab47e73566864 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1179,6 +1179,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> fc->no_control = ctx->no_control;
> fc->no_force_umount = ctx->no_force_umount;
> fc->no_mount_options = ctx->no_mount_options;
> + fc->init_security = ctx->init_security;
>
> err = -ENOMEM;
> root = fuse_get_root_inode(sb, ctx->rootmode);
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index bade747689033..ee22e9a8309df 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1051,6 +1051,7 @@ static int virtio_fs_fill_super(struct super_block *sb)
> .no_control = true,
> .no_force_umount = true,
> .no_mount_options = true,
> + .init_security = true,
> };
Should this is enabled from server instead (and not client). IIUC, one
of the deadlock examples stephen smalley gave was that client was waiting
for mount to finish and another getxattr() call went out. This will
succeed only if server is multi threaded and can handle both requests
in parallel. If that's the case should it be server which tells client
whether it can handle multiple parallel requests or not. If it can,
then client enables it.
Thanks
Vivek
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Virtio-fs] [PATCH] RFC: fuse: virtiofs: Call security hooks on new inodes
@ 2020-06-02 18:23 ` Vivek Goyal
0 siblings, 0 replies; 26+ messages in thread
From: Vivek Goyal @ 2020-06-02 18:23 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Miklos Szeredi, selinux, Suleiman Souhlal, virtio-fs,
linux-security-module, linux-fsdevel, Dylan Reid
On Mon, Jun 01, 2020 at 02:32:14PM +0900, Chirantan Ekbote wrote:
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Set this to true for
> virtiofs but false for regular fuse file systems.
>
> Calling security hooks is needed for `setfscreatecon` to work since it
> is applied as part of the selinux security hook.
>
> Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
> ---
> fs/fuse/dir.c | 74 ++++++++++++++++++++++++++++++++++++++++++---
> fs/fuse/fuse_i.h | 4 +++
> fs/fuse/inode.c | 1 +
> fs/fuse/virtio_fs.c | 1 +
> 4 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index de1e2fde60bd4..b18c92a8a4c11 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -16,6 +16,9 @@
> #include <linux/xattr.h>
> #include <linux/iversion.h>
> #include <linux/posix_acl.h>
> +#include <linux/security.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
>
> static void fuse_advise_use_readdirplus(struct inode *dir)
> {
> @@ -135,6 +138,50 @@ static void fuse_dir_changed(struct inode *dir)
> inode_maybe_inc_iversion(dir, false);
> }
>
> +static int fuse_initxattrs(struct inode *inode, const struct xattr *xattrs,
> + void *fs_info)
> +{
> + const struct xattr *xattr;
> + int err = 0;
> + int len;
> + char *name;
> +
> + for (xattr = xattrs; xattr->name != NULL; ++xattr) {
> + len = XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name) + 1;
> + name = kmalloc(len, GFP_KERNEL);
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> +
> + scnprintf(name, len, XATTR_SECURITY_PREFIX "%s", xattr->name);
> + err = fuse_setxattr(inode, name, xattr->value, xattr->value_len,
> + 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> +
> + return err;
> +}
> +
> +/*
> + * Initialize security on newly created inodes if supported by the filesystem.
> + */
> +static int fuse_init_security(struct inode *inode, struct inode *dir,
> + const struct qstr *qstr)
> +{
> + struct fuse_conn *conn = get_fuse_conn(dir);
> + int err = 0;
> +
> + if (conn->init_security) {
> + err = security_inode_init_security(inode, dir, qstr,
> + fuse_initxattrs, NULL);
> + }
> +
> + return err;
> +}
> +
> /**
> * Mark the attributes as stale due to an atime change. Avoid the invalidate if
> * atime is not used.
> @@ -498,7 +545,17 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> err = -ENOMEM;
> goto out_err;
> }
> +
> + err = fuse_init_security(inode, dir, &entry->d_name);
> + if (err) {
> + flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> + fi = get_fuse_inode(inode);
> + fuse_sync_release(fi, ff, flags);
> + fuse_queue_forget(fc, forget, outentry.nodeid, 1);
> + goto out_err;
> + }
> kfree(forget);
> +
[ cc lsm and selinux list ]
So this sets xattr after file creation. But this is not atomic w.r.t
file creation. I think keeping file creation and selinux context setting
to be atomic was one of the requirements.
Can we first retrieve the label which will be created for inode
(using dentry perhaps) and then pass that label as part of CREATE/MKNOD
request and then server can set fscreate (per thread) before file
creation. I hope /proc/[pid]/attr/fscreate work for per thread too.
Stephen had mentioned dentry_init_security() for this. Overlayfs uses
a variant of the same hook dentry_create_files_as().
> d_instantiate(entry, inode);
> fuse_change_entry_timeout(entry, &outentry);
> fuse_dir_changed(dir);
> @@ -569,7 +626,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
> */
> static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
> struct inode *dir, struct dentry *entry,
> - umode_t mode)
> + umode_t mode, bool init_security)
> {
> struct fuse_entry_out outarg;
> struct inode *inode;
> @@ -603,6 +660,13 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
> fuse_queue_forget(fc, forget, outarg.nodeid, 1);
> return -ENOMEM;
> }
> + if (init_security) {
> + err = fuse_init_security(inode, dir, &entry->d_name);
> + if (err) {
> + fuse_queue_forget(fc, forget, outarg.nodeid, 1);
> + return err;
> + }
> + }
> kfree(forget);
>
> d_drop(entry);
> @@ -644,7 +708,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, mode);
> + return create_new_entry(fc, &args, dir, entry, mode, true);
> }
>
> static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
> @@ -671,7 +735,7 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, S_IFDIR);
> + return create_new_entry(fc, &args, dir, entry, S_IFDIR, true);
> }
>
> static int fuse_symlink(struct inode *dir, struct dentry *entry,
> @@ -687,7 +751,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> args.in_args[0].value = entry->d_name.name;
> args.in_args[1].size = len;
> args.in_args[1].value = link;
> - return create_new_entry(fc, &args, dir, entry, S_IFLNK);
> + return create_new_entry(fc, &args, dir, entry, S_IFLNK, true);
> }
>
> void fuse_update_ctime(struct inode *inode)
> @@ -858,7 +922,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = newent->d_name.len + 1;
> args.in_args[1].value = newent->d_name.name;
> - err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
> + err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
> /* Contrary to "normal" filesystems it can happen that link
> makes two "logical" inodes point to the same "physical"
> inode. We invalidate the attributes of the old one, so it
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ca344bf714045..ed871742db584 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -482,6 +482,7 @@ struct fuse_fs_context {
> bool no_control:1;
> bool no_force_umount:1;
> bool no_mount_options:1;
> + bool init_security:1;
> unsigned int max_read;
> unsigned int blksize;
> const char *subtype;
> @@ -719,6 +720,9 @@ struct fuse_conn {
> /* Do not show mount options */
> unsigned int no_mount_options:1;
>
> + /* Initialize security xattrs when creating a new inode */
> + unsigned int init_security : 1;
> +
> /** The number of requests waiting for completion */
> atomic_t num_waiting;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 95d712d44ca13..ab47e73566864 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1179,6 +1179,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> fc->no_control = ctx->no_control;
> fc->no_force_umount = ctx->no_force_umount;
> fc->no_mount_options = ctx->no_mount_options;
> + fc->init_security = ctx->init_security;
>
> err = -ENOMEM;
> root = fuse_get_root_inode(sb, ctx->rootmode);
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index bade747689033..ee22e9a8309df 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1051,6 +1051,7 @@ static int virtio_fs_fill_super(struct super_block *sb)
> .no_control = true,
> .no_force_umount = true,
> .no_mount_options = true,
> + .init_security = true,
> };
Should this is enabled from server instead (and not client). IIUC, one
of the deadlock examples stephen smalley gave was that client was waiting
for mount to finish and another getxattr() call went out. This will
succeed only if server is multi threaded and can handle both requests
in parallel. If that's the case should it be server which tells client
whether it can handle multiple parallel requests or not. If it can,
then client enables it.
Thanks
Vivek
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] RFC: fuse: Call security hooks on new inodes
2020-06-01 5:32 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-06-10 9:27 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-10 9:27 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, Chirantan Ekbote
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Also add a
`FUSE_SECURITY_CTX` flag that can be set in the `flags` field of the
`fuse_init_out` struct that controls when the `init_security` field is
set.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests. The server should use this context by
writing it to `/proc/thread-self/attr/fscreate` before creating the
requested inode.
Calling security hooks is needed for `setfscreatecon` to work since it
is applied as part of the selinux security hook.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 99 +++++++++++++++++++++++++++++++++++++--
fs/fuse/fuse_i.h | 3 ++
fs/fuse/inode.c | 5 +-
include/uapi/linux/fuse.h | 8 +++-
4 files changed, 110 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..86bc073bb4f0a 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -629,6 +651,9 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
{
struct fuse_mknod_in inarg;
struct fuse_conn *fc = get_fuse_conn(dir);
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
if (!fc->dont_mask)
@@ -644,7 +669,27 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, mode);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -657,6 +702,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
{
struct fuse_mkdir_in inarg;
struct fuse_conn *fc = get_fuse_conn(dir);
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
if (!fc->dont_mask)
@@ -671,7 +719,28 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, S_IFDIR,
+ &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -679,6 +748,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
{
struct fuse_conn *fc = get_fuse_conn(dir);
unsigned len = strlen(link) + 1;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
args.opcode = FUSE_SYMLINK;
@@ -687,7 +759,28 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, S_IFLNK,
+ &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
void fuse_update_ctime(struct inode *inode)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ca344bf714045..5ea9212b0a71c 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -719,6 +719,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security : 1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..00919c214149d 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.278.ge193c7cf3a9-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Virtio-fs] [PATCH v2] RFC: fuse: Call security hooks on new inodes
@ 2020-06-10 9:27 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-10 9:27 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Suleiman Souhlal, virtio-fs, linux-fsdevel, Dylan Reid, Vivek Goyal
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Also add a
`FUSE_SECURITY_CTX` flag that can be set in the `flags` field of the
`fuse_init_out` struct that controls when the `init_security` field is
set.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests. The server should use this context by
writing it to `/proc/thread-self/attr/fscreate` before creating the
requested inode.
Calling security hooks is needed for `setfscreatecon` to work since it
is applied as part of the selinux security hook.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 99 +++++++++++++++++++++++++++++++++++++--
fs/fuse/fuse_i.h | 3 ++
fs/fuse/inode.c | 5 +-
include/uapi/linux/fuse.h | 8 +++-
4 files changed, 110 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..86bc073bb4f0a 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -629,6 +651,9 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
{
struct fuse_mknod_in inarg;
struct fuse_conn *fc = get_fuse_conn(dir);
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
if (!fc->dont_mask)
@@ -644,7 +669,27 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, mode);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -657,6 +702,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
{
struct fuse_mkdir_in inarg;
struct fuse_conn *fc = get_fuse_conn(dir);
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
if (!fc->dont_mask)
@@ -671,7 +719,28 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, S_IFDIR,
+ &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -679,6 +748,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
{
struct fuse_conn *fc = get_fuse_conn(dir);
unsigned len = strlen(link) + 1;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
+ int ret;
FUSE_ARGS(args);
args.opcode = FUSE_SYMLINK;
@@ -687,7 +759,28 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ if (fc->init_security) {
+ ret = security_dentry_init_security(entry, S_IFLNK,
+ &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (ret)
+ goto out;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
+ ret = create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ if (security_ctxlen > 0)
+ kfree(security_ctx);
+out:
+ return ret;
}
void fuse_update_ctime(struct inode *inode)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ca344bf714045..5ea9212b0a71c 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -719,6 +719,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security : 1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..00919c214149d 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.278.ge193c7cf3a9-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2] RFC: fuse: Call security hooks on new inodes
2020-06-10 9:27 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-06-15 7:37 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-15 7:37 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, Linux FS Devel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal
Friendly ping. Are there any concerns with this patch? I know I
probably need to split it up into 2 patches: one that adds the flag
and one that implements support. Since v2 adds a new flag to the
init_out struct I'd really like to get this merged upstream as
carrying it locally would effectively mean forking the protocol.
Thanks,
Chirantan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Virtio-fs] [PATCH v2] RFC: fuse: Call security hooks on new inodes
@ 2020-06-15 7:37 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-15 7:37 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Suleiman Souhlal, virtio-fs-list, Linux FS Devel, Dylan Reid,
Vivek Goyal
Friendly ping. Are there any concerns with this patch? I know I
probably need to split it up into 2 patches: one that adds the flag
and one that implements support. Since v2 adds a new flag to the
init_out struct I'd really like to get this merged upstream as
carrying it locally would effectively mean forking the protocol.
Thanks,
Chirantan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] RFC: fuse: Call security hooks on new inodes
2020-06-10 9:27 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-06-16 9:29 ` Miklos Szeredi
-1 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-06-16 9:29 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal
On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Also add a
> `FUSE_SECURITY_CTX` flag that can be set in the `flags` field of the
> `fuse_init_out` struct that controls when the `init_security` field is
> set.
>
> When set to true, get the security context for a newly created inode via
> `security_dentry_init_security` and append it to the create, mkdir,
> mknod, and symlink requests. The server should use this context by
> writing it to `/proc/thread-self/attr/fscreate` before creating the
> requested inode.
This is confusing. You mean if the server is stacking on top of a
real fs, then it can force the created new inode to have the given
security attributes by writing to that proc file?
> Calling security hooks is needed for `setfscreatecon` to work since it
> is applied as part of the selinux security hook.
>
> Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
> ---
> Changes in v2:
> * Added the FUSE_SECURITY_CTX flag for init_out responses.
> * Switched to security_dentry_init_security.
> * Send security context with create, mknod, mkdir, and symlink
> requests instead of applying it after creation.
>
> fs/fuse/dir.c | 99 +++++++++++++++++++++++++++++++++++++--
> fs/fuse/fuse_i.h | 3 ++
> fs/fuse/inode.c | 5 +-
> include/uapi/linux/fuse.h | 8 +++-
> 4 files changed, 110 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index ee190119f45cc..86bc073bb4f0a 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -16,6 +16,9 @@
> #include <linux/xattr.h>
> #include <linux/iversion.h>
> #include <linux/posix_acl.h>
> +#include <linux/security.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
>
> static void fuse_advise_use_readdirplus(struct inode *dir)
> {
> @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> struct fuse_entry_out outentry;
> struct fuse_inode *fi;
> struct fuse_file *ff;
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
>
> /* Userspace expects S_IFREG in create mode */
> BUG_ON((mode & S_IFMT) != S_IFREG);
> @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> args.out_args[0].value = &outentry;
> args.out_args[1].size = sizeof(outopen);
> args.out_args[1].value = &outopen;
> +
> + if (fc->init_security) {
> + err = security_dentry_init_security(entry, mode, &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (err)
> + goto out_put_forget_req;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
The above is quadruplicated, a helper is in order.
> err = fuse_simple_request(fc, &args);
> if (err)
> goto out_free_ff;
> @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> return err;
>
> out_free_ff:
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
== 0, then you need not check that condition.
> fuse_file_free(ff);
> out_put_forget_req:
> kfree(forget);
> @@ -629,6 +651,9 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> {
> struct fuse_mknod_in inarg;
> struct fuse_conn *fc = get_fuse_conn(dir);
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> if (!fc->dont_mask)
> @@ -644,7 +669,27 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, mode);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, mode, &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, mode);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
> @@ -657,6 +702,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> {
> struct fuse_mkdir_in inarg;
> struct fuse_conn *fc = get_fuse_conn(dir);
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> if (!fc->dont_mask)
> @@ -671,7 +719,28 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, S_IFDIR);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, S_IFDIR,
> + &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, S_IFDIR);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> static int fuse_symlink(struct inode *dir, struct dentry *entry,
> @@ -679,6 +748,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> {
> struct fuse_conn *fc = get_fuse_conn(dir);
> unsigned len = strlen(link) + 1;
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> args.opcode = FUSE_SYMLINK;
> @@ -687,7 +759,28 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> args.in_args[0].value = entry->d_name.name;
> args.in_args[1].size = len;
> args.in_args[1].value = link;
> - return create_new_entry(fc, &args, dir, entry, S_IFLNK);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, S_IFLNK,
> + &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, S_IFLNK);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> void fuse_update_ctime(struct inode *inode)
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ca344bf714045..5ea9212b0a71c 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -719,6 +719,9 @@ struct fuse_conn {
> /* Do not show mount options */
> unsigned int no_mount_options:1;
>
> + /* Initialize security xattrs when creating a new inode */
> + unsigned int init_security : 1;
> +
> /** The number of requests waiting for completion */
> atomic_t num_waiting;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 16aec32f7f3d7..1a311771c5555 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
> min_t(unsigned int, FUSE_MAX_MAX_PAGES,
> max_t(unsigned int, arg->max_pages, 1));
> }
> + if (arg->flags & FUSE_SECURITY_CTX)
> + fc->init_security = 1;
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> @@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
> FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
> FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
> FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
> - FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
> + FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
> + FUSE_SECURITY_CTX;
> ia->args.opcode = FUSE_INIT;
> ia->args.in_numargs = 1;
> ia->args.in_args[0].size = sizeof(ia->in);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 373cada898159..00919c214149d 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -172,6 +172,10 @@
> * - add FUSE_WRITE_KILL_PRIV flag
> * - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
> * - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
> + *
> + * 7.32
> + * - add FUSE_SECURITY_CTX flag for fuse_init_out
> + * - add security context to create, mkdir, and mknod requests
> */
>
> #ifndef _LINUX_FUSE_H
> @@ -207,7 +211,7 @@
> #define FUSE_KERNEL_VERSION 7
>
> /** Minor version number of this interface */
> -#define FUSE_KERNEL_MINOR_VERSION 31
> +#define FUSE_KERNEL_MINOR_VERSION 32
>
> /** The node ID of the root inode */
> #define FUSE_ROOT_ID 1
> @@ -314,6 +318,7 @@ struct fuse_file_lock {
> * FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
> * FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
> * FUSE_MAP_ALIGNMENT: map_alignment field is valid
> + * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
> */
> #define FUSE_ASYNC_READ (1 << 0)
> #define FUSE_POSIX_LOCKS (1 << 1)
> @@ -342,6 +347,7 @@ struct fuse_file_lock {
> #define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
> #define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
> #define FUSE_MAP_ALIGNMENT (1 << 26)
> +#define FUSE_SECURITY_CTX (1 << 27)
>
> /**
> * CUSE INIT request/reply flags
> --
> 2.27.0.278.ge193c7cf3a9-goog
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Virtio-fs] [PATCH v2] RFC: fuse: Call security hooks on new inodes
@ 2020-06-16 9:29 ` Miklos Szeredi
0 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-06-16 9:29 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Suleiman Souhlal, virtio-fs-list, linux-fsdevel, Dylan Reid, Vivek Goyal
On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Also add a
> `FUSE_SECURITY_CTX` flag that can be set in the `flags` field of the
> `fuse_init_out` struct that controls when the `init_security` field is
> set.
>
> When set to true, get the security context for a newly created inode via
> `security_dentry_init_security` and append it to the create, mkdir,
> mknod, and symlink requests. The server should use this context by
> writing it to `/proc/thread-self/attr/fscreate` before creating the
> requested inode.
This is confusing. You mean if the server is stacking on top of a
real fs, then it can force the created new inode to have the given
security attributes by writing to that proc file?
> Calling security hooks is needed for `setfscreatecon` to work since it
> is applied as part of the selinux security hook.
>
> Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
> ---
> Changes in v2:
> * Added the FUSE_SECURITY_CTX flag for init_out responses.
> * Switched to security_dentry_init_security.
> * Send security context with create, mknod, mkdir, and symlink
> requests instead of applying it after creation.
>
> fs/fuse/dir.c | 99 +++++++++++++++++++++++++++++++++++++--
> fs/fuse/fuse_i.h | 3 ++
> fs/fuse/inode.c | 5 +-
> include/uapi/linux/fuse.h | 8 +++-
> 4 files changed, 110 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index ee190119f45cc..86bc073bb4f0a 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -16,6 +16,9 @@
> #include <linux/xattr.h>
> #include <linux/iversion.h>
> #include <linux/posix_acl.h>
> +#include <linux/security.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
>
> static void fuse_advise_use_readdirplus(struct inode *dir)
> {
> @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> struct fuse_entry_out outentry;
> struct fuse_inode *fi;
> struct fuse_file *ff;
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
>
> /* Userspace expects S_IFREG in create mode */
> BUG_ON((mode & S_IFMT) != S_IFREG);
> @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> args.out_args[0].value = &outentry;
> args.out_args[1].size = sizeof(outopen);
> args.out_args[1].value = &outopen;
> +
> + if (fc->init_security) {
> + err = security_dentry_init_security(entry, mode, &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (err)
> + goto out_put_forget_req;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
The above is quadruplicated, a helper is in order.
> err = fuse_simple_request(fc, &args);
> if (err)
> goto out_free_ff;
> @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> return err;
>
> out_free_ff:
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
== 0, then you need not check that condition.
> fuse_file_free(ff);
> out_put_forget_req:
> kfree(forget);
> @@ -629,6 +651,9 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> {
> struct fuse_mknod_in inarg;
> struct fuse_conn *fc = get_fuse_conn(dir);
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> if (!fc->dont_mask)
> @@ -644,7 +669,27 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, mode);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, mode, &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, mode);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
> @@ -657,6 +702,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> {
> struct fuse_mkdir_in inarg;
> struct fuse_conn *fc = get_fuse_conn(dir);
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> if (!fc->dont_mask)
> @@ -671,7 +719,28 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
> args.in_args[0].value = &inarg;
> args.in_args[1].size = entry->d_name.len + 1;
> args.in_args[1].value = entry->d_name.name;
> - return create_new_entry(fc, &args, dir, entry, S_IFDIR);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, S_IFDIR,
> + &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, S_IFDIR);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> static int fuse_symlink(struct inode *dir, struct dentry *entry,
> @@ -679,6 +748,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> {
> struct fuse_conn *fc = get_fuse_conn(dir);
> unsigned len = strlen(link) + 1;
> + void *security_ctx = NULL;
> + u32 security_ctxlen = 0;
> + int ret;
> FUSE_ARGS(args);
>
> args.opcode = FUSE_SYMLINK;
> @@ -687,7 +759,28 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> args.in_args[0].value = entry->d_name.name;
> args.in_args[1].size = len;
> args.in_args[1].value = link;
> - return create_new_entry(fc, &args, dir, entry, S_IFLNK);
> +
> + if (fc->init_security) {
> + ret = security_dentry_init_security(entry, S_IFLNK,
> + &entry->d_name,
> + &security_ctx,
> + &security_ctxlen);
> + if (ret)
> + goto out;
> +
> + if (security_ctxlen > 0) {
> + args.in_numargs = 3;
> + args.in_args[2].size = security_ctxlen;
> + args.in_args[2].value = security_ctx;
> + }
> + }
> +
> + ret = create_new_entry(fc, &args, dir, entry, S_IFLNK);
> +
> + if (security_ctxlen > 0)
> + kfree(security_ctx);
> +out:
> + return ret;
> }
>
> void fuse_update_ctime(struct inode *inode)
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ca344bf714045..5ea9212b0a71c 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -719,6 +719,9 @@ struct fuse_conn {
> /* Do not show mount options */
> unsigned int no_mount_options:1;
>
> + /* Initialize security xattrs when creating a new inode */
> + unsigned int init_security : 1;
> +
> /** The number of requests waiting for completion */
> atomic_t num_waiting;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 16aec32f7f3d7..1a311771c5555 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
> min_t(unsigned int, FUSE_MAX_MAX_PAGES,
> max_t(unsigned int, arg->max_pages, 1));
> }
> + if (arg->flags & FUSE_SECURITY_CTX)
> + fc->init_security = 1;
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> @@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
> FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
> FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
> FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
> - FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
> + FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
> + FUSE_SECURITY_CTX;
> ia->args.opcode = FUSE_INIT;
> ia->args.in_numargs = 1;
> ia->args.in_args[0].size = sizeof(ia->in);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 373cada898159..00919c214149d 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -172,6 +172,10 @@
> * - add FUSE_WRITE_KILL_PRIV flag
> * - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
> * - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
> + *
> + * 7.32
> + * - add FUSE_SECURITY_CTX flag for fuse_init_out
> + * - add security context to create, mkdir, and mknod requests
> */
>
> #ifndef _LINUX_FUSE_H
> @@ -207,7 +211,7 @@
> #define FUSE_KERNEL_VERSION 7
>
> /** Minor version number of this interface */
> -#define FUSE_KERNEL_MINOR_VERSION 31
> +#define FUSE_KERNEL_MINOR_VERSION 32
>
> /** The node ID of the root inode */
> #define FUSE_ROOT_ID 1
> @@ -314,6 +318,7 @@ struct fuse_file_lock {
> * FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
> * FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
> * FUSE_MAP_ALIGNMENT: map_alignment field is valid
> + * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
> */
> #define FUSE_ASYNC_READ (1 << 0)
> #define FUSE_POSIX_LOCKS (1 << 1)
> @@ -342,6 +347,7 @@ struct fuse_file_lock {
> #define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
> #define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
> #define FUSE_MAP_ALIGNMENT (1 << 26)
> +#define FUSE_SECURITY_CTX (1 << 27)
>
> /**
> * CUSE INIT request/reply flags
> --
> 2.27.0.278.ge193c7cf3a9-goog
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] RFC: fuse: Call security hooks on new inodes
2020-06-16 9:29 ` [Virtio-fs] " Miklos Szeredi
@ 2020-06-16 9:41 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-16 9:41 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, Linux FS Devel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal
On Tue, Jun 16, 2020 at 6:29 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
> <chirantan@chromium.org> wrote:
> >
> >
> > When set to true, get the security context for a newly created inode via
> > `security_dentry_init_security` and append it to the create, mkdir,
> > mknod, and symlink requests. The server should use this context by
> > writing it to `/proc/thread-self/attr/fscreate` before creating the
> > requested inode.
>
> This is confusing. You mean if the server is stacking on top of a
> real fs, then it can force the created new inode to have the given
> security attributes by writing to that proc file?
>
Yes that's correct. Writing to that proc file ends up setting a field
in an selinux struct in the kernel. Later, when an inode is created
the selinux security hook uses that field to determine the label that
should be applied to the inode. This ensures that inodes appear
atomically with the correct selinux labels. Most users actually end
up using setfscreatecon from libselinux but all that does is write to
/proc/thread-self/attr/fscreate itself after doing some
conversion/validation.
> >
> > static void fuse_advise_use_readdirplus(struct inode *dir)
> > {
> > @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > struct fuse_entry_out outentry;
> > struct fuse_inode *fi;
> > struct fuse_file *ff;
> > + void *security_ctx = NULL;
> > + u32 security_ctxlen = 0;
> >
> > /* Userspace expects S_IFREG in create mode */
> > BUG_ON((mode & S_IFMT) != S_IFREG);
> > @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > args.out_args[0].value = &outentry;
> > args.out_args[1].size = sizeof(outopen);
> > args.out_args[1].value = &outopen;
> > +
> > + if (fc->init_security) {
> > + err = security_dentry_init_security(entry, mode, &entry->d_name,
> > + &security_ctx,
> > + &security_ctxlen);
> > + if (err)
> > + goto out_put_forget_req;
> > +
> > + if (security_ctxlen > 0) {
> > + args.in_numargs = 3;
> > + args.in_args[2].size = security_ctxlen;
> > + args.in_args[2].value = security_ctx;
> > + }
> > + }
> > +
>
> The above is quadruplicated, a helper is in order.
Ack.
>
> > err = fuse_simple_request(fc, &args);
> > if (err)
> > goto out_free_ff;
> > @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > return err;
> >
> > out_free_ff:
> > + if (security_ctxlen > 0)
> > + kfree(security_ctx);
>
> Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
> == 0, then you need not check that condition.
Ack. Will fix in v3.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Virtio-fs] [PATCH v2] RFC: fuse: Call security hooks on new inodes
@ 2020-06-16 9:41 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-06-16 9:41 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Suleiman Souhlal, virtio-fs-list, Linux FS Devel, Dylan Reid,
Vivek Goyal
On Tue, Jun 16, 2020 at 6:29 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
> <chirantan@chromium.org> wrote:
> >
> >
> > When set to true, get the security context for a newly created inode via
> > `security_dentry_init_security` and append it to the create, mkdir,
> > mknod, and symlink requests. The server should use this context by
> > writing it to `/proc/thread-self/attr/fscreate` before creating the
> > requested inode.
>
> This is confusing. You mean if the server is stacking on top of a
> real fs, then it can force the created new inode to have the given
> security attributes by writing to that proc file?
>
Yes that's correct. Writing to that proc file ends up setting a field
in an selinux struct in the kernel. Later, when an inode is created
the selinux security hook uses that field to determine the label that
should be applied to the inode. This ensures that inodes appear
atomically with the correct selinux labels. Most users actually end
up using setfscreatecon from libselinux but all that does is write to
/proc/thread-self/attr/fscreate itself after doing some
conversion/validation.
> >
> > static void fuse_advise_use_readdirplus(struct inode *dir)
> > {
> > @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > struct fuse_entry_out outentry;
> > struct fuse_inode *fi;
> > struct fuse_file *ff;
> > + void *security_ctx = NULL;
> > + u32 security_ctxlen = 0;
> >
> > /* Userspace expects S_IFREG in create mode */
> > BUG_ON((mode & S_IFMT) != S_IFREG);
> > @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > args.out_args[0].value = &outentry;
> > args.out_args[1].size = sizeof(outopen);
> > args.out_args[1].value = &outopen;
> > +
> > + if (fc->init_security) {
> > + err = security_dentry_init_security(entry, mode, &entry->d_name,
> > + &security_ctx,
> > + &security_ctxlen);
> > + if (err)
> > + goto out_put_forget_req;
> > +
> > + if (security_ctxlen > 0) {
> > + args.in_numargs = 3;
> > + args.in_args[2].size = security_ctxlen;
> > + args.in_args[2].value = security_ctx;
> > + }
> > + }
> > +
>
> The above is quadruplicated, a helper is in order.
Ack.
>
> > err = fuse_simple_request(fc, &args);
> > if (err)
> > goto out_free_ff;
> > @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > return err;
> >
> > out_free_ff:
> > + if (security_ctxlen > 0)
> > + kfree(security_ctx);
>
> Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
> == 0, then you need not check that condition.
Ack. Will fix in v3.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2] RFC: fuse: Call security hooks on new inodes
2020-06-16 9:41 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-06-16 10:27 ` Miklos Szeredi
-1 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-06-16 10:27 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Vivek Goyal, Stefan Hajnoczi, Linux FS Devel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal
On Tue, Jun 16, 2020 at 11:41 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> On Tue, Jun 16, 2020 at 6:29 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
> > <chirantan@chromium.org> wrote:
> > >
> > >
> > > When set to true, get the security context for a newly created inode via
> > > `security_dentry_init_security` and append it to the create, mkdir,
> > > mknod, and symlink requests. The server should use this context by
> > > writing it to `/proc/thread-self/attr/fscreate` before creating the
> > > requested inode.
> >
> > This is confusing. You mean if the server is stacking on top of a
> > real fs, then it can force the created new inode to have the given
> > security attributes by writing to that proc file?
> >
>
> Yes that's correct. Writing to that proc file ends up setting a field
> in an selinux struct in the kernel. Later, when an inode is created
> the selinux security hook uses that field to determine the label that
> should be applied to the inode. This ensures that inodes appear
> atomically with the correct selinux labels. Most users actually end
> up using setfscreatecon from libselinux but all that does is write to
> /proc/thread-self/attr/fscreate itself after doing some
> conversion/validation.
FUSE servers do not necessarily use a real filesystem as a backing
store (e.g. network filesystems), so you should clarify that in the
description.
>
> > >
> > > static void fuse_advise_use_readdirplus(struct inode *dir)
> > > {
> > > @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > struct fuse_entry_out outentry;
> > > struct fuse_inode *fi;
> > > struct fuse_file *ff;
> > > + void *security_ctx = NULL;
> > > + u32 security_ctxlen = 0;
> > >
> > > /* Userspace expects S_IFREG in create mode */
> > > BUG_ON((mode & S_IFMT) != S_IFREG);
> > > @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > args.out_args[0].value = &outentry;
> > > args.out_args[1].size = sizeof(outopen);
> > > args.out_args[1].value = &outopen;
> > > +
> > > + if (fc->init_security) {
> > > + err = security_dentry_init_security(entry, mode, &entry->d_name,
> > > + &security_ctx,
> > > + &security_ctxlen);
> > > + if (err)
> > > + goto out_put_forget_req;
> > > +
> > > + if (security_ctxlen > 0) {
> > > + args.in_numargs = 3;
> > > + args.in_args[2].size = security_ctxlen;
> > > + args.in_args[2].value = security_ctx;
> > > + }
> > > + }
> > > +
> >
> > The above is quadruplicated, a helper is in order.
>
> Ack.
>
> >
> > > err = fuse_simple_request(fc, &args);
> > > if (err)
> > > goto out_free_ff;
> > > @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > return err;
> > >
> > > out_free_ff:
> > > + if (security_ctxlen > 0)
> > > + kfree(security_ctx);
> >
> > Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
> > == 0, then you need not check that condition.
>
> Ack. Will fix in v3.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Virtio-fs] [PATCH v2] RFC: fuse: Call security hooks on new inodes
@ 2020-06-16 10:27 ` Miklos Szeredi
0 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-06-16 10:27 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Suleiman Souhlal, virtio-fs-list, Linux FS Devel, Dylan Reid,
Vivek Goyal
On Tue, Jun 16, 2020 at 11:41 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> On Tue, Jun 16, 2020 at 6:29 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, Jun 10, 2020 at 11:27 AM Chirantan Ekbote
> > <chirantan@chromium.org> wrote:
> > >
> > >
> > > When set to true, get the security context for a newly created inode via
> > > `security_dentry_init_security` and append it to the create, mkdir,
> > > mknod, and symlink requests. The server should use this context by
> > > writing it to `/proc/thread-self/attr/fscreate` before creating the
> > > requested inode.
> >
> > This is confusing. You mean if the server is stacking on top of a
> > real fs, then it can force the created new inode to have the given
> > security attributes by writing to that proc file?
> >
>
> Yes that's correct. Writing to that proc file ends up setting a field
> in an selinux struct in the kernel. Later, when an inode is created
> the selinux security hook uses that field to determine the label that
> should be applied to the inode. This ensures that inodes appear
> atomically with the correct selinux labels. Most users actually end
> up using setfscreatecon from libselinux but all that does is write to
> /proc/thread-self/attr/fscreate itself after doing some
> conversion/validation.
FUSE servers do not necessarily use a real filesystem as a backing
store (e.g. network filesystems), so you should clarify that in the
description.
>
> > >
> > > static void fuse_advise_use_readdirplus(struct inode *dir)
> > > {
> > > @@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > struct fuse_entry_out outentry;
> > > struct fuse_inode *fi;
> > > struct fuse_file *ff;
> > > + void *security_ctx = NULL;
> > > + u32 security_ctxlen = 0;
> > >
> > > /* Userspace expects S_IFREG in create mode */
> > > BUG_ON((mode & S_IFMT) != S_IFREG);
> > > @@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > args.out_args[0].value = &outentry;
> > > args.out_args[1].size = sizeof(outopen);
> > > args.out_args[1].value = &outopen;
> > > +
> > > + if (fc->init_security) {
> > > + err = security_dentry_init_security(entry, mode, &entry->d_name,
> > > + &security_ctx,
> > > + &security_ctxlen);
> > > + if (err)
> > > + goto out_put_forget_req;
> > > +
> > > + if (security_ctxlen > 0) {
> > > + args.in_numargs = 3;
> > > + args.in_args[2].size = security_ctxlen;
> > > + args.in_args[2].value = security_ctx;
> > > + }
> > > + }
> > > +
> >
> > The above is quadruplicated, a helper is in order.
>
> Ack.
>
> >
> > > err = fuse_simple_request(fc, &args);
> > > if (err)
> > > goto out_free_ff;
> > > @@ -513,6 +533,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> > > return err;
> > >
> > > out_free_ff:
> > > + if (security_ctxlen > 0)
> > > + kfree(security_ctx);
> >
> > Freeing NULL is okay, if that's guaranteed in case of security_ctxlen
> > == 0, then you need not check that condition.
>
> Ack. Will fix in v3.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 1/2] uapi: fuse: Add FUSE_SECURITY_CTX
2020-06-10 9:27 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-13 9:09 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:09 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, fuse-devel, Chirantan Ekbote
Add the FUSE_SECURITY_CTX flag for the `flags` field of the
fuse_init_out struct. When this flag is set the kernel will append the
security context for a newly created inode to the request (create,
mkdir, mknod, and symlink). The server is responsible for ensuring that
the inode appears atomically with the requested security context.
For example, if the server is backed by a "real" linux file system then
it can write the security context value to
/proc/thread-self/attr/fscreate before making the syscall to create the
inode.
---
include/uapi/linux/fuse.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..e2099b45fd44b 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, symlink, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Virtio-fs] [PATCHv3 1/2] uapi: fuse: Add FUSE_SECURITY_CTX
@ 2020-07-13 9:09 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:09 UTC (permalink / raw)
To: Miklos Szeredi
Cc: fuse-devel, Suleiman Souhlal, virtio-fs, linux-fsdevel,
Dylan Reid, Vivek Goyal
Add the FUSE_SECURITY_CTX flag for the `flags` field of the
fuse_init_out struct. When this flag is set the kernel will append the
security context for a newly created inode to the request (create,
mkdir, mknod, and symlink). The server is responsible for ensuring that
the inode appears atomically with the requested security context.
For example, if the server is backed by a "real" linux file system then
it can write the security context value to
/proc/thread-self/attr/fscreate before making the syscall to create the
inode.
---
include/uapi/linux/fuse.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..e2099b45fd44b 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, symlink, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv3 2/2] fuse: Call security hooks on new inodes
2020-07-13 9:09 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-13 9:09 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:09 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, fuse-devel, Chirantan Ekbote
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Set this to true when
the `flags` field of the fuse_init_out struct contains
FUSE_SECURITY_CTX.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests.
---
Changes in v3:
* Moved uapi changes into a separate patch.
* Refactored duplicated common code into create_new_entry.
* Dropped check if security_ctxlen > 0 since kfree can handle NULL.
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 3 +++
fs/fuse/inode.c | 5 +++-
3 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..b4045dc55bccd 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -569,13 +590,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
*/
static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
struct inode *dir, struct dentry *entry,
- umode_t mode)
+ umode_t mode, bool init_security)
{
struct fuse_entry_out outarg;
struct inode *inode;
struct dentry *d;
int err;
struct fuse_forget_link *forget;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
forget = fuse_alloc_forget();
if (!forget)
@@ -586,7 +609,29 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
args->out_numargs = 1;
args->out_args[0].size = sizeof(outarg);
args->out_args[0].value = &outarg;
+
+ if (init_security) {
+ unsigned short idx = args->in_numargs;
+ if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+ return -ENOMEM;
+ }
+
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ return err;
+
+ if (security_ctxlen > 0) {
+ args->in_args[idx].size = security_ctxlen;
+ args->in_args[idx].value = security_ctx;
+ args->in_numargs++;
+ }
+ }
+
err = fuse_simple_request(fc, args);
+ kfree(security_ctx);
+
if (err)
goto out_put_forget_req;
@@ -644,7 +689,8 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ return create_new_entry(fc, &args, dir, entry, mode, fc->init_security);
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -671,7 +717,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFDIR,
+ fc->init_security);
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -687,7 +735,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFLNK,
+ fc->init_security);
}
void fuse_update_ctime(struct inode *inode)
@@ -858,7 +908,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
args.in_args[0].value = &inarg;
args.in_args[1].size = newent->d_name.len + 1;
args.in_args[1].value = newent->d_name.name;
- err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
+ err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
/* Contrary to "normal" filesystems it can happen that link
makes two "logical" inodes point to the same "physical"
inode. We invalidate the attributes of the old one, so it
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d7cde216fc871..dd7422d83da3d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -720,6 +720,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security:1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Virtio-fs] [PATCHv3 2/2] fuse: Call security hooks on new inodes
@ 2020-07-13 9:09 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:09 UTC (permalink / raw)
To: Miklos Szeredi
Cc: fuse-devel, Suleiman Souhlal, virtio-fs, linux-fsdevel,
Dylan Reid, Vivek Goyal
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Set this to true when
the `flags` field of the fuse_init_out struct contains
FUSE_SECURITY_CTX.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests.
---
Changes in v3:
* Moved uapi changes into a separate patch.
* Refactored duplicated common code into create_new_entry.
* Dropped check if security_ctxlen > 0 since kfree can handle NULL.
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 3 +++
fs/fuse/inode.c | 5 +++-
3 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..b4045dc55bccd 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -569,13 +590,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
*/
static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
struct inode *dir, struct dentry *entry,
- umode_t mode)
+ umode_t mode, bool init_security)
{
struct fuse_entry_out outarg;
struct inode *inode;
struct dentry *d;
int err;
struct fuse_forget_link *forget;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
forget = fuse_alloc_forget();
if (!forget)
@@ -586,7 +609,29 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
args->out_numargs = 1;
args->out_args[0].size = sizeof(outarg);
args->out_args[0].value = &outarg;
+
+ if (init_security) {
+ unsigned short idx = args->in_numargs;
+ if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+ return -ENOMEM;
+ }
+
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ return err;
+
+ if (security_ctxlen > 0) {
+ args->in_args[idx].size = security_ctxlen;
+ args->in_args[idx].value = security_ctx;
+ args->in_numargs++;
+ }
+ }
+
err = fuse_simple_request(fc, args);
+ kfree(security_ctx);
+
if (err)
goto out_put_forget_req;
@@ -644,7 +689,8 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ return create_new_entry(fc, &args, dir, entry, mode, fc->init_security);
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -671,7 +717,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFDIR,
+ fc->init_security);
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -687,7 +735,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFLNK,
+ fc->init_security);
}
void fuse_update_ctime(struct inode *inode)
@@ -858,7 +908,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
args.in_args[0].value = &inarg;
args.in_args[1].size = newent->d_name.len + 1;
args.in_args[1].value = newent->d_name.name;
- err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
+ err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
/* Contrary to "normal" filesystems it can happen that link
makes two "logical" inodes point to the same "physical"
inode. We invalidate the attributes of the old one, so it
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d7cde216fc871..dd7422d83da3d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -720,6 +720,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security:1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCHv4 1/2] uapi: fuse: Add FUSE_SECURITY_CTX
2020-07-13 9:09 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-13 9:56 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:56 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, fuse-devel, Chirantan Ekbote
Add the FUSE_SECURITY_CTX flag for the `flags` field of the
fuse_init_out struct. When this flag is set the kernel will append the
security context for a newly created inode to the request (create,
mkdir, mknod, and symlink). The server is responsible for ensuring that
the inode appears atomically with the requested security context.
For example, if the server is backed by a "real" linux file system then
it can write the security context value to
/proc/thread-self/attr/fscreate before making the syscall to create the
inode.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v4:
* Added signoff to commit message.
include/uapi/linux/fuse.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..e2099b45fd44b 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, symlink, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Virtio-fs] [PATCHv4 1/2] uapi: fuse: Add FUSE_SECURITY_CTX
@ 2020-07-13 9:56 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:56 UTC (permalink / raw)
To: Miklos Szeredi
Cc: fuse-devel, Suleiman Souhlal, virtio-fs, linux-fsdevel,
Dylan Reid, Vivek Goyal
Add the FUSE_SECURITY_CTX flag for the `flags` field of the
fuse_init_out struct. When this flag is set the kernel will append the
security context for a newly created inode to the request (create,
mkdir, mknod, and symlink). The server is responsible for ensuring that
the inode appears atomically with the requested security context.
For example, if the server is backed by a "real" linux file system then
it can write the security context value to
/proc/thread-self/attr/fscreate before making the syscall to create the
inode.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v4:
* Added signoff to commit message.
include/uapi/linux/fuse.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 373cada898159..e2099b45fd44b 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -172,6 +172,10 @@
* - add FUSE_WRITE_KILL_PRIV flag
* - add FUSE_SETUPMAPPING and FUSE_REMOVEMAPPING
* - add map_alignment to fuse_init_out, add FUSE_MAP_ALIGNMENT flag
+ *
+ * 7.32
+ * - add FUSE_SECURITY_CTX flag for fuse_init_out
+ * - add security context to create, mkdir, symlink, and mknod requests
*/
#ifndef _LINUX_FUSE_H
@@ -207,7 +211,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 31
+#define FUSE_KERNEL_MINOR_VERSION 32
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -314,6 +318,7 @@ struct fuse_file_lock {
* FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
* FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
* FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_SECURITY_CTX: add security context to create, mkdir, symlink, and mknod
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -342,6 +347,7 @@ struct fuse_file_lock {
#define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
#define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
#define FUSE_MAP_ALIGNMENT (1 << 26)
+#define FUSE_SECURITY_CTX (1 << 27)
/**
* CUSE INIT request/reply flags
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv4 2/2] fuse: Call security hooks on new inodes
2020-07-13 9:56 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-13 9:57 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:57 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, linux-fsdevel, virtio-fs,
Dylan Reid, Suleiman Souhlal, fuse-devel, Chirantan Ekbote
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Set this to true when
the `flags` field of the fuse_init_out struct contains
FUSE_SECURITY_CTX.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v4:
* Added signoff to commit message.
* Fixed style warnings reported by checkpatch.pl.
Changes in v3:
* Moved uapi changes into a separate patch.
* Refactored duplicated common code into create_new_entry.
* Dropped check if security_ctxlen > 0 since kfree can handle NULL.
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 3 +++
fs/fuse/inode.c | 5 +++-
3 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..c6791c49afe4d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -569,13 +590,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
*/
static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
struct inode *dir, struct dentry *entry,
- umode_t mode)
+ umode_t mode, bool init_security)
{
struct fuse_entry_out outarg;
struct inode *inode;
struct dentry *d;
int err;
struct fuse_forget_link *forget;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
forget = fuse_alloc_forget();
if (!forget)
@@ -586,7 +609,29 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
args->out_numargs = 1;
args->out_args[0].size = sizeof(outarg);
args->out_args[0].value = &outarg;
+
+ if (init_security) {
+ unsigned short idx = args->in_numargs;
+
+ if ((size_t)idx >= ARRAY_SIZE(args->in_args))
+ return -ENOMEM;
+
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ return err;
+
+ if (security_ctxlen > 0) {
+ args->in_args[idx].size = security_ctxlen;
+ args->in_args[idx].value = security_ctx;
+ args->in_numargs++;
+ }
+ }
+
err = fuse_simple_request(fc, args);
+ kfree(security_ctx);
+
if (err)
goto out_put_forget_req;
@@ -644,7 +689,8 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ return create_new_entry(fc, &args, dir, entry, mode, fc->init_security);
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -671,7 +717,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFDIR,
+ fc->init_security);
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -687,7 +735,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFLNK,
+ fc->init_security);
}
void fuse_update_ctime(struct inode *inode)
@@ -858,7 +908,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
args.in_args[0].value = &inarg;
args.in_args[1].size = newent->d_name.len + 1;
args.in_args[1].value = newent->d_name.name;
- err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
+ err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
/* Contrary to "normal" filesystems it can happen that link
makes two "logical" inodes point to the same "physical"
inode. We invalidate the attributes of the old one, so it
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d7cde216fc871..dd7422d83da3d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -720,6 +720,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security : 1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Virtio-fs] [PATCHv4 2/2] fuse: Call security hooks on new inodes
@ 2020-07-13 9:57 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-13 9:57 UTC (permalink / raw)
To: Miklos Szeredi
Cc: fuse-devel, Suleiman Souhlal, virtio-fs, linux-fsdevel,
Dylan Reid, Vivek Goyal
Add a new `init_security` field to `fuse_conn` that controls whether we
initialize security when a new inode is created. Set this to true when
the `flags` field of the fuse_init_out struct contains
FUSE_SECURITY_CTX.
When set to true, get the security context for a newly created inode via
`security_dentry_init_security` and append it to the create, mkdir,
mknod, and symlink requests.
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
Changes in v4:
* Added signoff to commit message.
* Fixed style warnings reported by checkpatch.pl.
Changes in v3:
* Moved uapi changes into a separate patch.
* Refactored duplicated common code into create_new_entry.
* Dropped check if security_ctxlen > 0 since kfree can handle NULL.
Changes in v2:
* Added the FUSE_SECURITY_CTX flag for init_out responses.
* Switched to security_dentry_init_security.
* Send security context with create, mknod, mkdir, and symlink
requests instead of applying it after creation.
fs/fuse/dir.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
fs/fuse/fuse_i.h | 3 +++
fs/fuse/inode.c | 5 +++-
3 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ee190119f45cc..c6791c49afe4d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -16,6 +16,9 @@
#include <linux/xattr.h>
#include <linux/iversion.h>
#include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
static void fuse_advise_use_readdirplus(struct inode *dir)
{
@@ -442,6 +445,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
struct fuse_entry_out outentry;
struct fuse_inode *fi;
struct fuse_file *ff;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
/* Userspace expects S_IFREG in create mode */
BUG_ON((mode & S_IFMT) != S_IFREG);
@@ -477,6 +482,21 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
args.out_args[0].value = &outentry;
args.out_args[1].size = sizeof(outopen);
args.out_args[1].value = &outopen;
+
+ if (fc->init_security) {
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ goto out_put_forget_req;
+
+ if (security_ctxlen > 0) {
+ args.in_numargs = 3;
+ args.in_args[2].size = security_ctxlen;
+ args.in_args[2].value = security_ctx;
+ }
+ }
+
err = fuse_simple_request(fc, &args);
if (err)
goto out_free_ff;
@@ -513,6 +533,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
return err;
out_free_ff:
+ kfree(security_ctx);
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
@@ -569,13 +590,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
*/
static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
struct inode *dir, struct dentry *entry,
- umode_t mode)
+ umode_t mode, bool init_security)
{
struct fuse_entry_out outarg;
struct inode *inode;
struct dentry *d;
int err;
struct fuse_forget_link *forget;
+ void *security_ctx = NULL;
+ u32 security_ctxlen = 0;
forget = fuse_alloc_forget();
if (!forget)
@@ -586,7 +609,29 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_args *args,
args->out_numargs = 1;
args->out_args[0].size = sizeof(outarg);
args->out_args[0].value = &outarg;
+
+ if (init_security) {
+ unsigned short idx = args->in_numargs;
+
+ if ((size_t)idx >= ARRAY_SIZE(args->in_args))
+ return -ENOMEM;
+
+ err = security_dentry_init_security(entry, mode, &entry->d_name,
+ &security_ctx,
+ &security_ctxlen);
+ if (err)
+ return err;
+
+ if (security_ctxlen > 0) {
+ args->in_args[idx].size = security_ctxlen;
+ args->in_args[idx].value = security_ctx;
+ args->in_numargs++;
+ }
+ }
+
err = fuse_simple_request(fc, args);
+ kfree(security_ctx);
+
if (err)
goto out_put_forget_req;
@@ -644,7 +689,8 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, mode);
+
+ return create_new_entry(fc, &args, dir, entry, mode, fc->init_security);
}
static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
@@ -671,7 +717,9 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
args.in_args[0].value = &inarg;
args.in_args[1].size = entry->d_name.len + 1;
args.in_args[1].value = entry->d_name.name;
- return create_new_entry(fc, &args, dir, entry, S_IFDIR);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFDIR,
+ fc->init_security);
}
static int fuse_symlink(struct inode *dir, struct dentry *entry,
@@ -687,7 +735,9 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
args.in_args[0].value = entry->d_name.name;
args.in_args[1].size = len;
args.in_args[1].value = link;
- return create_new_entry(fc, &args, dir, entry, S_IFLNK);
+
+ return create_new_entry(fc, &args, dir, entry, S_IFLNK,
+ fc->init_security);
}
void fuse_update_ctime(struct inode *inode)
@@ -858,7 +908,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
args.in_args[0].value = &inarg;
args.in_args[1].size = newent->d_name.len + 1;
args.in_args[1].value = newent->d_name.name;
- err = create_new_entry(fc, &args, newdir, newent, inode->i_mode);
+ err = create_new_entry(fc, &args, newdir, newent, inode->i_mode, false);
/* Contrary to "normal" filesystems it can happen that link
makes two "logical" inodes point to the same "physical"
inode. We invalidate the attributes of the old one, so it
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d7cde216fc871..dd7422d83da3d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -720,6 +720,9 @@ struct fuse_conn {
/* Do not show mount options */
unsigned int no_mount_options:1;
+ /* Initialize security xattrs when creating a new inode */
+ unsigned int init_security : 1;
+
/** The number of requests waiting for completion */
atomic_t num_waiting;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 16aec32f7f3d7..1a311771c5555 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -951,6 +951,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
min_t(unsigned int, FUSE_MAX_MAX_PAGES,
max_t(unsigned int, arg->max_pages, 1));
}
+ if (arg->flags & FUSE_SECURITY_CTX)
+ fc->init_security = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -988,7 +990,8 @@ void fuse_send_init(struct fuse_conn *fc)
FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
- FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
+ FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
+ FUSE_SECURITY_CTX;
ia->args.opcode = FUSE_INIT;
ia->args.in_numargs = 1;
ia->args.in_args[0].size = sizeof(ia->in);
--
2.27.0.383.g050319c2ae-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCHv4 2/2] fuse: Call security hooks on new inodes
2020-07-13 9:57 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-21 8:07 ` Chirantan Ekbote
-1 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-21 8:07 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Vivek Goyal, Stefan Hajnoczi, Linux FS Devel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal, fuse-devel
On Mon, Jul 13, 2020 at 6:57 PM Chirantan Ekbote <chirantan@chromium.org> wrote:
>
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Set this to true when
> the `flags` field of the fuse_init_out struct contains
> FUSE_SECURITY_CTX.
>
> When set to true, get the security context for a newly created inode via
> `security_dentry_init_security` and append it to the create, mkdir,
> mknod, and symlink requests.
>
Are there any other concerns with this patch? Or can I expect that it
will get merged eventually?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Virtio-fs] [PATCHv4 2/2] fuse: Call security hooks on new inodes
@ 2020-07-21 8:07 ` Chirantan Ekbote
0 siblings, 0 replies; 26+ messages in thread
From: Chirantan Ekbote @ 2020-07-21 8:07 UTC (permalink / raw)
To: Miklos Szeredi
Cc: fuse-devel, Suleiman Souhlal, virtio-fs-list, Linux FS Devel,
Dylan Reid, Vivek Goyal
On Mon, Jul 13, 2020 at 6:57 PM Chirantan Ekbote <chirantan@chromium.org> wrote:
>
> Add a new `init_security` field to `fuse_conn` that controls whether we
> initialize security when a new inode is created. Set this to true when
> the `flags` field of the fuse_init_out struct contains
> FUSE_SECURITY_CTX.
>
> When set to true, get the security context for a newly created inode via
> `security_dentry_init_security` and append it to the create, mkdir,
> mknod, and symlink requests.
>
Are there any other concerns with this patch? Or can I expect that it
will get merged eventually?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv4 2/2] fuse: Call security hooks on new inodes
2020-07-21 8:07 ` [Virtio-fs] " Chirantan Ekbote
@ 2020-07-21 14:23 ` Miklos Szeredi
-1 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-07-21 14:23 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: Vivek Goyal, Stefan Hajnoczi, Linux FS Devel, virtio-fs-list,
Dylan Reid, Suleiman Souhlal, fuse-devel
On Tue, Jul 21, 2020 at 10:07 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> On Mon, Jul 13, 2020 at 6:57 PM Chirantan Ekbote <chirantan@chromium.org> wrote:
> >
> > Add a new `init_security` field to `fuse_conn` that controls whether we
> > initialize security when a new inode is created. Set this to true when
> > the `flags` field of the fuse_init_out struct contains
> > FUSE_SECURITY_CTX.
> >
> > When set to true, get the security context for a newly created inode via
> > `security_dentry_init_security` and append it to the create, mkdir,
> > mknod, and symlink requests.
> >
>
> Are there any other concerns with this patch? Or can I expect that it
> will get merged eventually?
Looks good to me. Can you resend with the security/selinux folks in the CC?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Virtio-fs] [PATCHv4 2/2] fuse: Call security hooks on new inodes
@ 2020-07-21 14:23 ` Miklos Szeredi
0 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2020-07-21 14:23 UTC (permalink / raw)
To: Chirantan Ekbote
Cc: fuse-devel, Suleiman Souhlal, virtio-fs-list, Linux FS Devel,
Dylan Reid, Vivek Goyal
On Tue, Jul 21, 2020 at 10:07 AM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> On Mon, Jul 13, 2020 at 6:57 PM Chirantan Ekbote <chirantan@chromium.org> wrote:
> >
> > Add a new `init_security` field to `fuse_conn` that controls whether we
> > initialize security when a new inode is created. Set this to true when
> > the `flags` field of the fuse_init_out struct contains
> > FUSE_SECURITY_CTX.
> >
> > When set to true, get the security context for a newly created inode via
> > `security_dentry_init_security` and append it to the create, mkdir,
> > mknod, and symlink requests.
> >
>
> Are there any other concerns with this patch? Or can I expect that it
> will get merged eventually?
Looks good to me. Can you resend with the security/selinux folks in the CC?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 26+ messages in thread