linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] udf: augment owner permissions on new inodes
@ 2019-08-19 14:27 Steven J. Magnani
  2019-08-26 10:14 ` Jan Kara
  0 siblings, 1 reply; 2+ messages in thread
From: Steven J. Magnani @ 2019-08-19 14:27 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-kernel, linux-fsdevel, Steven J . Magnani

Windows presents files created within Linux as read-only, even when
permissions in Linux indicate the file should be writable.


UDF defines a slightly different set of basic file permissions than Linux.
Specifically, UDF has "delete" and "change attribute" permissions for each
access class (user/group/other). Linux has no equivalents for these.

When the Linux UDF driver creates a file (or directory), no UDF delete or
change attribute permissions are granted. The lack of delete permission
appears to cause Windows to mark an item read-only when its permissions
otherwise indicate that it should be read-write.

Fix this by granting UDF delete and change attribute permissions
to the owner when creating a new inode.

Reported by: Ty Young
Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
--- a/fs/udf/udf_i.h	2019-08-14 07:24:05.029508342 -0500
+++ b/fs/udf/udf_i.h	2019-08-19 08:55:37.797394177 -0500
@@ -38,6 +38,7 @@ struct udf_inode_info {
 	__u32			i_next_alloc_block;
 	__u32			i_next_alloc_goal;
 	__u32			i_checkpoint;
+	__u32			i_extraPerms;
 	unsigned		i_alloc_type : 3;
 	unsigned		i_efe : 1;	/* extendedFileEntry */
 	unsigned		i_use : 1;	/* unallocSpaceEntry */
--- a/fs/udf/ialloc.c	2019-08-14 07:24:05.029508342 -0500
+++ b/fs/udf/ialloc.c	2019-08-19 08:33:08.992422457 -0500
@@ -118,6 +118,7 @@ struct inode *udf_new_inode(struct inode
 	iinfo->i_lenAlloc = 0;
 	iinfo->i_use = 0;
 	iinfo->i_checkpoint = 1;
+	iinfo->i_extraPerms = FE_PERM_U_DELETE | FE_PERM_U_CHATTR;
 	if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
 		iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
 	else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
--- a/fs/udf/inode.c	2019-08-14 07:24:05.029508342 -0500
+++ b/fs/udf/inode.c	2019-08-19 08:42:46.537530051 -0500
@@ -45,6 +45,10 @@
 
 #define EXTENT_MERGE_SIZE 5
 
+#define FE_MAPPED_PERMS	(FE_PERM_U_READ | FE_PERM_U_WRITE | FE_PERM_U_EXEC | \
+			 FE_PERM_G_READ | FE_PERM_G_WRITE | FE_PERM_G_EXEC | \
+			 FE_PERM_O_READ | FE_PERM_O_WRITE | FE_PERM_O_EXEC)
+
 static umode_t udf_convert_permissions(struct fileEntry *);
 static int udf_update_inode(struct inode *, int);
 static int udf_sync_inode(struct inode *inode);
@@ -1458,6 +1462,8 @@ reread:
 	else
 		inode->i_mode = udf_convert_permissions(fe);
 	inode->i_mode &= ~sbi->s_umask;
+	iinfo->i_extraPerms = le32_to_cpu(fe->permissions) & ~FE_MAPPED_PERMS;
+
 	read_unlock(&sbi->s_cred_lock);
 
 	link_count = le16_to_cpu(fe->fileLinkCount);
@@ -1691,10 +1697,7 @@ static int udf_update_inode(struct inode
 		   ((inode->i_mode & 0070) << 2) |
 		   ((inode->i_mode & 0700) << 4);
 
-	udfperms |= (le32_to_cpu(fe->permissions) &
-		    (FE_PERM_O_DELETE | FE_PERM_O_CHATTR |
-		     FE_PERM_G_DELETE | FE_PERM_G_CHATTR |
-		     FE_PERM_U_DELETE | FE_PERM_U_CHATTR));
+	udfperms |= iinfo->i_extraPerms;
 	fe->permissions = cpu_to_le32(udfperms);
 
 	if (S_ISDIR(inode->i_mode) && inode->i_nlink > 0)

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] udf: augment owner permissions on new inodes
  2019-08-19 14:27 [PATCH] udf: augment owner permissions on new inodes Steven J. Magnani
@ 2019-08-26 10:14 ` Jan Kara
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2019-08-26 10:14 UTC (permalink / raw)
  To:  Steven J. Magnani 
  Cc: Jan Kara, Steven J . Magnani, linux-fsdevel, linux-kernel

On Mon 19-08-19 09:27:07,  Steven J. Magnani  wrote:
> Windows presents files created within Linux as read-only, even when
> permissions in Linux indicate the file should be writable.
> 
> 
> UDF defines a slightly different set of basic file permissions than Linux.
> Specifically, UDF has "delete" and "change attribute" permissions for each
> access class (user/group/other). Linux has no equivalents for these.
> 
> When the Linux UDF driver creates a file (or directory), no UDF delete or
> change attribute permissions are granted. The lack of delete permission
> appears to cause Windows to mark an item read-only when its permissions
> otherwise indicate that it should be read-write.
> 
> Fix this by granting UDF delete and change attribute permissions
> to the owner when creating a new inode.
> 
> Reported by: Ty Young
> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>

Thanks for the patch! The behavior for CHATTR and DELETE permissions is
defined by UDF specification in 3.3.3.3 section. From that I'd say that we
should set CHATTR for user on creation and otherwise leave it untouched
(which is what you seem to be doing). For DELETE permission we should set
it to match WRITE permission (needs handling on file create in in
udf_setattr() when changing file permissions). Can you please fixup the
DELETE permission behavior?

								Honza

> ---
> --- a/fs/udf/udf_i.h	2019-08-14 07:24:05.029508342 -0500
> +++ b/fs/udf/udf_i.h	2019-08-19 08:55:37.797394177 -0500
> @@ -38,6 +38,7 @@ struct udf_inode_info {
>  	__u32			i_next_alloc_block;
>  	__u32			i_next_alloc_goal;
>  	__u32			i_checkpoint;
> +	__u32			i_extraPerms;
>  	unsigned		i_alloc_type : 3;
>  	unsigned		i_efe : 1;	/* extendedFileEntry */
>  	unsigned		i_use : 1;	/* unallocSpaceEntry */
> --- a/fs/udf/ialloc.c	2019-08-14 07:24:05.029508342 -0500
> +++ b/fs/udf/ialloc.c	2019-08-19 08:33:08.992422457 -0500
> @@ -118,6 +118,7 @@ struct inode *udf_new_inode(struct inode
>  	iinfo->i_lenAlloc = 0;
>  	iinfo->i_use = 0;
>  	iinfo->i_checkpoint = 1;
> +	iinfo->i_extraPerms = FE_PERM_U_DELETE | FE_PERM_U_CHATTR;
>  	if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
>  		iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
>  	else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
> --- a/fs/udf/inode.c	2019-08-14 07:24:05.029508342 -0500
> +++ b/fs/udf/inode.c	2019-08-19 08:42:46.537530051 -0500
> @@ -45,6 +45,10 @@
>  
>  #define EXTENT_MERGE_SIZE 5
>  
> +#define FE_MAPPED_PERMS	(FE_PERM_U_READ | FE_PERM_U_WRITE | FE_PERM_U_EXEC | \
> +			 FE_PERM_G_READ | FE_PERM_G_WRITE | FE_PERM_G_EXEC | \
> +			 FE_PERM_O_READ | FE_PERM_O_WRITE | FE_PERM_O_EXEC)
> +
>  static umode_t udf_convert_permissions(struct fileEntry *);
>  static int udf_update_inode(struct inode *, int);
>  static int udf_sync_inode(struct inode *inode);
> @@ -1458,6 +1462,8 @@ reread:
>  	else
>  		inode->i_mode = udf_convert_permissions(fe);
>  	inode->i_mode &= ~sbi->s_umask;
> +	iinfo->i_extraPerms = le32_to_cpu(fe->permissions) & ~FE_MAPPED_PERMS;
> +
>  	read_unlock(&sbi->s_cred_lock);
>  
>  	link_count = le16_to_cpu(fe->fileLinkCount);
> @@ -1691,10 +1697,7 @@ static int udf_update_inode(struct inode
>  		   ((inode->i_mode & 0070) << 2) |
>  		   ((inode->i_mode & 0700) << 4);
>  
> -	udfperms |= (le32_to_cpu(fe->permissions) &
> -		    (FE_PERM_O_DELETE | FE_PERM_O_CHATTR |
> -		     FE_PERM_G_DELETE | FE_PERM_G_CHATTR |
> -		     FE_PERM_U_DELETE | FE_PERM_U_CHATTR));
> +	udfperms |= iinfo->i_extraPerms;
>  	fe->permissions = cpu_to_le32(udfperms);
>  
>  	if (S_ISDIR(inode->i_mode) && inode->i_nlink > 0)
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-08-26 10:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 14:27 [PATCH] udf: augment owner permissions on new inodes Steven J. Magnani
2019-08-26 10:14 ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).