Re: [PATCH] fuse: allow umask processing in userspace

["Jean-Pierre André" <jean-pierre.andre@wanadoo.fr>]

Hi Miklos,

Good news : use of default ACL is now ok,
Bad news : several tests on mkdir/mknod fail.

This is because umask is not applied anymore, in
any situation, and it was somewhat expected as I
defined current_umask() as zero.

BUT :

I only built a new fuse.ko for kernel 2.6.29.5-191
based on the fuse source released with this kernel
and the below patches applied (manually). No change
whatever elsewhere (I did not recompile the kernel),
and there is nothing in this patch which removes the
old code which was applying the umask (there is only
new code which applies the umask on condition).

Also I did not update the server part (the file system),
which should be ok as the old fields remain at the same
location (and the new behaviour cannot be requested).

So why does this patch prevent the umask being
applied to the "mode" argument ? I mean : how does
it avoid the old code which was applying the umask ?

Is there a hidden hack somewhere ?

Regards

Jean-Pierre





Miklos Szeredi wrote:
> On Fri, 19 Jun 2009, Jean-Pierre André wrote:
>   
>> On the very related issue of having some way of getting
>> the original permission flags (with umask not applied),
>> in create(), has there been any progress ?
>>     
>
> OK, here's a patch that does this, it applies on top of latest git.
> Could you please give it a test?
>
> Even if you are not using git, you can get a "snapshot" of the git
> tree from www.kernel.org.
>
> I'll follow up with a patch for libfuse (also against latest CVS or
> 2.8.0-pre3)
>
> Thanks,
> Miklos
>
> ---
> This patch lets filesystems handle masking the file mode on creation.
> This is needed if filesystem is using ACLs.
>
>  - The CREATE, MKDIR and MKNOD requests are extended with a "umask"
>    parameter.
>
>  - A new FUSE_DONT_MASK flag is added to the INIT request/reply.  With
>    this the filesystem may request that the create mode is not masked.
>
> CC: Jean-Pierre André <jean-pierre.andre@wanadoo.fr>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
>  fs/fuse/dir.c        |   20 +++++++++++++++++---
>  fs/fuse/fuse_i.h     |    3 +++
>  fs/fuse/inode.c      |    9 ++++++++-
>  include/linux/fuse.h |   20 ++++++++++++++++++--
>  4 files changed, 46 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/fs/fuse/dir.c
> ===================================================================
> --- linux-2.6.orig/fs/fuse/dir.c	2009-06-22 20:37:44.000000000 +0200
> +++ linux-2.6/fs/fuse/dir.c	2009-06-23 10:06:36.000000000 +0200
> @@ -375,7 +375,7 @@ static int fuse_create_open(struct inode
>  	struct fuse_conn *fc = get_fuse_conn(dir);
>  	struct fuse_req *req;
>  	struct fuse_req *forget_req;
> -	struct fuse_open_in inarg;
> +	struct fuse_create_in inarg;
>  	struct fuse_open_out outopen;
>  	struct fuse_entry_out outentry;
>  	struct fuse_file *ff;
> @@ -399,15 +399,20 @@ static int fuse_create_open(struct inode
>  	if (!ff)
>  		goto out_put_request;
>  
> +	if (!fc->dont_mask)
> +		mode &= ~current_umask();
> +
>  	flags &= ~O_NOCTTY;
>  	memset(&inarg, 0, sizeof(inarg));
>  	memset(&outentry, 0, sizeof(outentry));
>  	inarg.flags = flags;
>  	inarg.mode = mode;
> +	inarg.umask = current_umask();
>  	req->in.h.opcode = FUSE_CREATE;
>  	req->in.h.nodeid = get_node_id(dir);
>  	req->in.numargs = 2;
> -	req->in.args[0].size = sizeof(inarg);
> +	req->in.args[0].size = fc->minor < 12 ? sizeof(struct fuse_open_in) :
> +						sizeof(inarg);
>  	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;
> @@ -546,12 +551,17 @@ static int fuse_mknod(struct inode *dir,
>  	if (IS_ERR(req))
>  		return PTR_ERR(req);
>  
> +	if (!fc->dont_mask)
> +		mode &= ~current_umask();
> +
>  	memset(&inarg, 0, sizeof(inarg));
>  	inarg.mode = mode;
>  	inarg.rdev = new_encode_dev(rdev);
> +	inarg.umask = current_umask();
>  	req->in.h.opcode = FUSE_MKNOD;
>  	req->in.numargs = 2;
> -	req->in.args[0].size = sizeof(inarg);
> +	req->in.args[0].size = fc->minor < 12 ? FUSE_COMPAT_MKNOD_IN_SIZE :
> +						sizeof(inarg);
>  	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;
> @@ -578,8 +588,12 @@ static int fuse_mkdir(struct inode *dir,
>  	if (IS_ERR(req))
>  		return PTR_ERR(req);
>  
> +	if (!fc->dont_mask)
> +		mode &= ~current_umask();
> +
>  	memset(&inarg, 0, sizeof(inarg));
>  	inarg.mode = mode;
> +	inarg.umask = current_umask();
>  	req->in.h.opcode = FUSE_MKDIR;
>  	req->in.numargs = 2;
>  	req->in.args[0].size = sizeof(inarg);
> Index: linux-2.6/fs/fuse/fuse_i.h
> ===================================================================
> --- linux-2.6.orig/fs/fuse/fuse_i.h	2009-06-22 20:37:44.000000000 +0200
> +++ linux-2.6/fs/fuse/fuse_i.h	2009-06-22 20:42:59.000000000 +0200
> @@ -446,6 +446,9 @@ struct fuse_conn {
>  	/** Do multi-page cached writes */
>  	unsigned big_writes:1;
>  
> +	/** Don't apply umask to creation modes */
> +	unsigned dont_mask:1;
> +
>  	/** The number of requests waiting for completion */
>  	atomic_t num_waiting;
>  
> Index: linux-2.6/fs/fuse/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/fuse/inode.c	2009-06-22 20:37:44.000000000 +0200
> +++ linux-2.6/fs/fuse/inode.c	2009-06-22 20:51:10.000000000 +0200
> @@ -725,6 +725,8 @@ static void process_init_reply(struct fu
>  			}
>  			if (arg->flags & FUSE_BIG_WRITES)
>  				fc->big_writes = 1;
> +			if (arg->flags & FUSE_DONT_MASK)
> +				fc->dont_mask = 1;
>  		} else {
>  			ra_pages = fc->max_read / PAGE_CACHE_SIZE;
>  			fc->no_lock = 1;
> @@ -748,7 +750,7 @@ static void fuse_send_init(struct fuse_c
>  	arg->minor = FUSE_KERNEL_MINOR_VERSION;
>  	arg->max_readahead = fc->bdi.ra_pages * PAGE_CACHE_SIZE;
>  	arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC |
> -		FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES;
> +		FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK;
>  	req->in.h.opcode = FUSE_INIT;
>  	req->in.numargs = 1;
>  	req->in.args[0].size = sizeof(*arg);
> @@ -864,6 +866,11 @@ static int fuse_fill_super(struct super_
>  	if (err)
>  		goto err_put_conn;
>  
> +	/* Handle umasking inside the fuse code */
> +	if (sb->s_flags & MS_POSIXACL)
> +		fc->dont_mask = 1;
> +	sb->s_flags |= MS_POSIXACL;
> +
>  	fc->release = fuse_free_conn;
>  	fc->flags = d.flags;
>  	fc->user_id = d.user_id;
> Index: linux-2.6/include/linux/fuse.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fuse.h	2009-06-22 20:37:44.000000000 +0200
> +++ linux-2.6/include/linux/fuse.h	2009-06-23 10:05:45.000000000 +0200
> @@ -25,6 +25,9 @@
>   *  - add IOCTL message
>   *  - add unsolicited notification support
>   *  - add POLL message and NOTIFY_POLL notification
> + *
> + * 7.12
> + *  - add umask flag to input argument of open, mknod and mkdir
>   */
>  
>  #ifndef _LINUX_FUSE_H
> @@ -36,7 +39,7 @@
>  #define FUSE_KERNEL_VERSION 7
>  
>  /** Minor version number of this interface */
> -#define FUSE_KERNEL_MINOR_VERSION 11
> +#define FUSE_KERNEL_MINOR_VERSION 12
>  
>  /** The node ID of the root inode */
>  #define FUSE_ROOT_ID 1
> @@ -112,6 +115,7 @@ struct fuse_file_lock {
>   * INIT request/reply flags
>   *
>   * FUSE_EXPORT_SUPPORT: filesystem handles lookups of "." and ".."
> + * FUSE_DONT_MASK: don't apply umask to file mode on create operations
>   */
>  #define FUSE_ASYNC_READ		(1 << 0)
>  #define FUSE_POSIX_LOCKS	(1 << 1)
> @@ -119,6 +123,7 @@ struct fuse_file_lock {
>  #define FUSE_ATOMIC_O_TRUNC	(1 << 3)
>  #define FUSE_EXPORT_SUPPORT	(1 << 4)
>  #define FUSE_BIG_WRITES		(1 << 5)
> +#define FUSE_DONT_MASK		(1 << 6)
>  
>  /**
>   * CUSE INIT request/reply flags
> @@ -262,14 +267,18 @@ struct fuse_attr_out {
>  	struct fuse_attr attr;
>  };
>  
> +#define FUSE_COMPAT_MKNOD_IN_SIZE 8
> +
>  struct fuse_mknod_in {
>  	__u32	mode;
>  	__u32	rdev;
> +	__u32	umask;
> +	__u32	padding;
>  };
>  
>  struct fuse_mkdir_in {
>  	__u32	mode;
> -	__u32	padding;
> +	__u32	umask;
>  };
>  
>  struct fuse_rename_in {
> @@ -301,7 +310,14 @@ struct fuse_setattr_in {
>  
>  struct fuse_open_in {
>  	__u32	flags;
> +	__u32	unused;
> +};
> +
> +struct fuse_create_in {
> +	__u32	flags;
>  	__u32	mode;
> +	__u32	umask;
> +	__u32	padding;
>  };
>  
>  struct fuse_open_out {
>
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html