Menu
▾
▴
Re: [fuse-devel] [PATCH v2 2/3] fuse: selinux extended attribute label support
|
From: Brian F. <bf...@re...> - 2014-01-06 22:16:46
|
On 01/06/2014 10:47 AM, Miklos Szeredi wrote:
> On Mon, Nov 25, 2013 at 09:22:22AM -0500, Brian Foster wrote:
>> Define the FUSE_SELINUX_XATTR init bit to advertise selinux client
>> label support. When enabled, fuse initializes security on all newly
>> allocated inodes.
>>
>> By specifying FUSE_SELINUX_XATTR, the client fs indicates it is
>> safe to probe for getxattr support,
>
> I don't quite understand. Selinux does some probing on mount, yes? And that
> tends to lock up fuse filesystems that are unprepared. What was the exact
> mechanism of the deadlock and how do you work around it in the userspace
> fs?
>
Hi Miklos,
Thanks for taking a look at this. This was addressed in gluster some
time ago. I didn't make the fix so I don't have the specific failure
details, but from looking at the code this was addressed in general by
fork()'ing a child for the mount() call and thus allowing the fuse
request processing thread to initialize and handle requests before the
mount() might have completed. E.g., it polls on both a mount status pipe
(written to with the mount() results) and the fuse fd to handle mount
status and/or incoming requests simultaneously.
> More comments inline...
>
>> the selinux policy is
>> appropriately configured for fuse and that it intends to handle the
>> additional security.selinux.* extended attribute requests in
>> whatever manner is appropriate for the particular fs.
>>
>> Signed-off-by: Brian Foster <bf...@re...>
>> ---
>> fs/fuse/dir.c | 72 +++++++++++++++++++++++++++++++++++++++++++----
>> fs/fuse/fuse_i.h | 3 ++
>> fs/fuse/inode.c | 5 +++-
>> include/uapi/linux/fuse.h | 7 ++++-
>> 4 files changed, 80 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index 8153f59..b16e04a 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -13,6 +13,11 @@
>> #include <linux/sched.h>
>> #include <linux/namei.h>
>> #include <linux/slab.h>
>> +#include <linux/xattr.h>
>> +#include <linux/security.h>
>> +
>> +static int fuse_init_security(struct inode *, struct inode *,
>> + const struct qstr *);
>>
>> static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
>> {
>> @@ -470,6 +475,13 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
>> goto out_err;
>> }
>> kfree(forget);
>> +
>> + err = fuse_init_security(inode, dir, &entry->d_name);
>> + if (err < 0) {
>> + iput(inode);
>> + return err;
>> + }
>> +
>
> What happens if called twice on the same inode?
>
> It shouldn't happen on well behaved filesystems, but with fuse the general
> assumption is that the userspace filesystem may not always behave well (by
> returning an existing inode in create, for example). The kernel must take
> these without problem.
>
>> d_instantiate(entry, inode);
>> fuse_change_entry_timeout(entry, &outentry);
>> fuse_invalidate_attr(dir);
>> @@ -541,7 +553,7 @@ no_open:
>> */
>> static int create_new_entry(struct fuse_conn *fc, struct fuse_req *req,
>> struct inode *dir, struct dentry *entry,
>> - umode_t mode)
>> + umode_t mode, int init_sec)
>> {
>> struct fuse_entry_out outarg;
>> struct inode *inode;
>> @@ -583,6 +595,14 @@ static int create_new_entry(struct fuse_conn *fc, struct fuse_req *req,
>> }
>> kfree(forget);
>>
>> + if (init_sec) {
>> + err = fuse_init_security(inode, dir, &entry->d_name);
>> + if (err < 0) {
>> + iput(inode);
>> + return err;
>> + }
>> + }
>> +
>
> Same here. You assume that a newly created inode will be unique and that a
> linked inode will be pre-existing. But neither of those conditions can safely
> be relied on with fuse.
>
Hmm, good points. I'm going to have to look into the resulting behavior
here and then think about this a bit more. Thanks.
Brian
>> err = d_instantiate_no_diralias(entry, inode);
>> if (err)
>> return err;
>> @@ -619,7 +639,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, umode_t mode,
>> req->in.args[0].value = &inarg;
>> req->in.args[1].size = entry->d_name.len + 1;
>> req->in.args[1].value = entry->d_name.name;
>> - return create_new_entry(fc, req, dir, entry, mode);
>> + return create_new_entry(fc, req, dir, entry, mode, 1);
>> }
>>
>> static int fuse_create(struct inode *dir, struct dentry *entry, umode_t mode,
>> @@ -648,7 +668,7 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, umode_t mode)
>> req->in.args[0].value = &inarg;
>> req->in.args[1].size = entry->d_name.len + 1;
>> req->in.args[1].value = entry->d_name.name;
>> - return create_new_entry(fc, req, dir, entry, S_IFDIR);
>> + return create_new_entry(fc, req, dir, entry, S_IFDIR, 1);
>> }
>>
>> static int fuse_symlink(struct inode *dir, struct dentry *entry,
>> @@ -666,7 +686,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
>> req->in.args[0].value = entry->d_name.name;
>> req->in.args[1].size = len;
>> req->in.args[1].value = link;
>> - return create_new_entry(fc, req, dir, entry, S_IFLNK);
>> + return create_new_entry(fc, req, dir, entry, S_IFLNK, 1);
>> }
>>
>> static int fuse_unlink(struct inode *dir, struct dentry *entry)
>> @@ -804,7 +824,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
>> req->in.args[0].value = &inarg;
>> req->in.args[1].size = newent->d_name.len + 1;
>> req->in.args[1].value = newent->d_name.name;
>> - err = create_new_entry(fc, req, newdir, newent, inode->i_mode);
>> + err = create_new_entry(fc, req, newdir, newent, inode->i_mode, 0);
>> /* 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
>> @@ -1923,6 +1943,48 @@ static const struct inode_operations fuse_symlink_inode_operations = {
>> .removexattr = fuse_removexattr,
>> };
>>
>> +static int fuse_initxattrs(struct inode *inode, const struct xattr *xattr_array,
>> + void *fs_info)
>> +{
>> + const struct xattr *xattr;
>> + int error = 0;
>> + int len;
>> + char *name;
>> +
>> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>> + len = XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name) + 1;
>> + name = kmalloc(len, GFP_KERNEL);
>> + if (!name) {
>> + error = -ENOMEM;
>> + break;
>> + }
>> + snprintf(name, len, XATTR_SECURITY_PREFIX "%s", xattr->name);
>> + error = __fuse_setxattr(inode, name, xattr->value,
>> + xattr->value_len, 0);
>> + kfree(name);
>> + if (error < 0)
>> + break;
>> + }
>> +
>> + return error;
>> +}
>> +
>> +/*
>> + * Initialize security on the newly created inode if the userspace fs supports
>> + * client labelling via extended attributes.
>> + */
>> +static int fuse_init_security(struct inode *inode, struct inode *dir,
>> + const struct qstr *qstr)
>> +{
>> + struct fuse_conn *fc = get_fuse_conn(dir);
>> +
>> + if (!fc->selinux_xattr)
>> + return 0;
>> +
>> + return security_inode_init_security(inode, dir, qstr, &fuse_initxattrs,
>> + NULL);
>> +}
>> +
>> void fuse_init_common(struct inode *inode)
>> {
>> inode->i_op = &fuse_common_inode_operations;
>> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
>> index 7d27309..f03d77f 100644
>> --- a/fs/fuse/fuse_i.h
>> +++ b/fs/fuse/fuse_i.h
>> @@ -548,6 +548,9 @@ struct fuse_conn {
>> /** Does the filesystem support asynchronous direct-IO submission? */
>> unsigned async_dio:1;
>>
>> + /** Does the filesystem support selinux via extended attributes? */
>> + unsigned selinux_xattr: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 d468643..6d6489a 100644
>> --- a/fs/fuse/inode.c
>> +++ b/fs/fuse/inode.c
>> @@ -873,6 +873,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
>> }
>> if (arg->flags & FUSE_ASYNC_DIO)
>> fc->async_dio = 1;
>> + if (arg->flags & FUSE_SELINUX_XATTR)
>> + fc->selinux_xattr = 1;
>> } else {
>> ra_pages = fc->max_read / PAGE_CACHE_SIZE;
>> fc->no_lock = 1;
>> @@ -900,7 +902,8 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
>> FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK |
>> FUSE_SPLICE_WRITE | FUSE_SPLICE_MOVE | FUSE_SPLICE_READ |
>> FUSE_FLOCK_LOCKS | FUSE_IOCTL_DIR | FUSE_AUTO_INVAL_DATA |
>> - FUSE_DO_READDIRPLUS | FUSE_READDIRPLUS_AUTO | FUSE_ASYNC_DIO;
>> + FUSE_DO_READDIRPLUS | FUSE_READDIRPLUS_AUTO | FUSE_ASYNC_DIO |
>> + FUSE_SELINUX_XATTR;
>> req->in.h.opcode = FUSE_INIT;
>> req->in.numargs = 1;
>> req->in.args[0].size = sizeof(*arg);
>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
>> index 60bb2f9..326898c 100644
>> --- a/include/uapi/linux/fuse.h
>> +++ b/include/uapi/linux/fuse.h
>> @@ -93,6 +93,9 @@
>> *
>> * 7.22
>> * - add FUSE_ASYNC_DIO
>> + *
>> + * 7.23
>> + * - add FUSE_SELINUX_XATTR
>> */
>>
>> #ifndef _LINUX_FUSE_H
>> @@ -128,7 +131,7 @@
>> #define FUSE_KERNEL_VERSION 7
>>
>> /** Minor version number of this interface */
>> -#define FUSE_KERNEL_MINOR_VERSION 22
>> +#define FUSE_KERNEL_MINOR_VERSION 23
>>
>> /** The node ID of the root inode */
>> #define FUSE_ROOT_ID 1
>> @@ -219,6 +222,7 @@ struct fuse_file_lock {
>> * FUSE_DO_READDIRPLUS: do READDIRPLUS (READDIR+LOOKUP in one)
>> * FUSE_READDIRPLUS_AUTO: adaptive readdirplus
>> * FUSE_ASYNC_DIO: asynchronous direct I/O submission
>> + * FUSE_SELINUX_XATTR: selinux extended attribute support
>> */
>> #define FUSE_ASYNC_READ (1 << 0)
>> #define FUSE_POSIX_LOCKS (1 << 1)
>> @@ -236,6 +240,7 @@ struct fuse_file_lock {
>> #define FUSE_DO_READDIRPLUS (1 << 13)
>> #define FUSE_READDIRPLUS_AUTO (1 << 14)
>> #define FUSE_ASYNC_DIO (1 << 15)
>> +#define FUSE_SELINUX_XATTR (1 << 16)
>>
>> /**
>> * CUSE INIT request/reply flags
>> --
>> 1.8.1.4
>>
>>
>> ------------------------------------------------------------------------------
>> Shape the Mobile Experience: Free Subscription
>> Software experts and developers: Be at the forefront of tech innovation.
>> Intel(R) Software Adrenaline delivers strategic insight and game-changing
>> conversations that shape the rapidly evolving mobile landscape. Sign up now.
>> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
>> _______________________________________________
>> fuse-devel mailing list
>> fus...@li...
>> https://lists.sourceforge.net/lists/listinfo/fuse-devel
|