From: Jan Kara <jack@suse.cz>
To: " Steven J. Magnani " <steve.magnani@digidescorp.com>
Cc: Jan Kara <jack@suse.com>,
"Steven J . Magnani" <steve@digidescorp.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] udf: augment owner permissions on new inodes
Date: Mon, 26 Aug 2019 12:14:38 +0200 [thread overview]
Message-ID: <20190826101438.GC10614@quack2.suse.cz> (raw)
In-Reply-To: <20190819142707.18070-1-steve@digidescorp.com>
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
prev parent reply other threads:[~2019-08-26 10:14 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-19 14:27 [PATCH] udf: augment owner permissions on new inodes Steven J. Magnani
2019-08-26 10:14 ` Jan Kara [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190826101438.GC10614@quack2.suse.cz \
--to=jack@suse.cz \
--cc=jack@suse.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=steve.magnani@digidescorp.com \
--cc=steve@digidescorp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).