[PATCH v2] exfat: add ioctls for accessing attributes

[PATCH v2] exfat: add ioctls for accessing attributes

[Jan Cincera]

Add GET and SET attributes ioctls to enable attribute modification.
We already do this in FAT and a few userspace utils made for it would
benefit from this also working on exFAT, namely fatattr.

Signed-off-by: Jan Cincera <hcincera@gmail.com>
---
Changes in v2:
  - Removed irrelevant comments.
  - Now masking reserved fields.

 fs/exfat/exfat_fs.h |  6 +++
 fs/exfat/file.c     | 93 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 729ada9e26e8..ebe8c4b928f4 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -149,6 +149,12 @@ enum {
 #define DIR_CACHE_SIZE		\
 	(DIV_ROUND_UP(EXFAT_DEN_TO_B(ES_MAX_ENTRY_NUM), SECTOR_SIZE) + 1)
 
+/*
+ * attribute ioctls, same as their FAT equivalents.
+ */
+#define EXFAT_IOCTL_GET_ATTRIBUTES	_IOR('r', 0x10, __u32)
+#define EXFAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u32)
+
 struct exfat_dentry_namebuf {
 	char *lfn;
 	int lfnbuf_len; /* usually MAX_UNINAME_BUF_SIZE */
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 3cbd270e0cba..b31ce0868ddd 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -8,6 +8,8 @@
 #include <linux/cred.h>
 #include <linux/buffer_head.h>
 #include <linux/blkdev.h>
+#include <linux/fsnotify.h>
+#include <linux/security.h>
 
 #include "exfat_raw.h"
 #include "exfat_fs.h"
@@ -316,6 +318,92 @@ int exfat_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	return error;
 }
 
+/*
+ * modified ioctls from fat/file.c by Welmer Almesberger
+ */
+static int exfat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
+{
+	u32 attr;
+
+	inode_lock_shared(inode);
+	attr = exfat_make_attr(inode);
+	inode_unlock_shared(inode);
+
+	return put_user(attr, user_attr);
+}
+
+static int exfat_ioctl_set_attributes(struct file *file, u32 __user *user_attr)
+{
+	struct inode *inode = file_inode(file);
+	struct exfat_sb_info *sbi = EXFAT_SB(inode->i_sb);
+	int is_dir = S_ISDIR(inode->i_mode);
+	u32 attr, oldattr;
+	struct iattr ia;
+	int err;
+
+	err = get_user(attr, user_attr);
+	if (err)
+		goto out;
+
+	err = mnt_want_write_file(file);
+	if (err)
+		goto out;
+	inode_lock(inode);
+
+	oldattr = exfat_make_attr(inode);
+
+	/*
+	 * Mask attributes so we don't set reserved fields.
+	 */
+	attr &= (ATTR_READONLY | ATTR_HIDDEN | ATTR_SYSTEM | ATTR_ARCHIVE);
+	attr |= (is_dir ? ATTR_SUBDIR : 0);
+
+	/* Equivalent to a chmod() */
+	ia.ia_valid = ATTR_MODE | ATTR_CTIME;
+	ia.ia_ctime = current_time(inode);
+	if (is_dir)
+		ia.ia_mode = exfat_make_mode(sbi, attr, 0777);
+	else
+		ia.ia_mode = exfat_make_mode(sbi, attr, 0666 | (inode->i_mode & 0111));
+
+	/* The root directory has no attributes */
+	if (inode->i_ino == EXFAT_ROOT_INO && attr != ATTR_SUBDIR) {
+		err = -EINVAL;
+		goto out_unlock_inode;
+	}
+
+	if (((attr | oldattr) & ATTR_SYSTEM) &&
+	    !capable(CAP_LINUX_IMMUTABLE)) {
+		err = -EPERM;
+		goto out_unlock_inode;
+	}
+
+	/*
+	 * The security check is questionable...  We single
+	 * out the RO attribute for checking by the security
+	 * module, just because it maps to a file mode.
+	 */
+	err = security_inode_setattr(file_mnt_idmap(file),
+				     file->f_path.dentry, &ia);
+	if (err)
+		goto out_unlock_inode;
+
+	/* This MUST be done before doing anything irreversible... */
+	err = exfat_setattr(file_mnt_idmap(file), file->f_path.dentry, &ia);
+	if (err)
+		goto out_unlock_inode;
+
+	fsnotify_change(file->f_path.dentry, ia.ia_valid);
+
+	exfat_save_attr(inode, attr);
+	mark_inode_dirty(inode);
+out_unlock_inode:
+	inode_unlock(inode);
+	mnt_drop_write_file(file);
+out:
+	return err;
+}
+
 static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg)
 {
 	struct fstrim_range range;
@@ -346,8 +434,13 @@ static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg)
 long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct inode *inode = file_inode(filp);
+	u32 __user *user_attr = (u32 __user *)arg;
 
 	switch (cmd) {
+	case EXFAT_IOCTL_GET_ATTRIBUTES:
+		return exfat_ioctl_get_attributes(inode, user_attr);
+	case EXFAT_IOCTL_SET_ATTRIBUTES:
+		return exfat_ioctl_set_attributes(filp, user_attr);
 	case FITRIM:
 		return exfat_ioctl_fitrim(inode, arg);
 	default:
-- 
2.40.1

Re: [PATCH v2] exfat: add ioctls for accessing attributes

[Darrick J. Wong]

On Sun, Aug 27, 2023 at 12:42:07PM +0200, Jan Cincera wrote:
> Add GET and SET attributes ioctls to enable attribute modification.
> We already do this in FAT and a few userspace utils made for it would
> benefit from this also working on exFAT, namely fatattr.
> 
> Signed-off-by: Jan Cincera <hcincera@gmail.com>
> ---
> Changes in v2:
>   - Removed irrelevant comments.
>   - Now masking reserved fields.
> 
>  fs/exfat/exfat_fs.h |  6 +++
>  fs/exfat/file.c     | 93 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 99 insertions(+)
> 
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> index 729ada9e26e8..ebe8c4b928f4 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -149,6 +149,12 @@ enum {
>  #define DIR_CACHE_SIZE		\
>  	(DIV_ROUND_UP(EXFAT_DEN_TO_B(ES_MAX_ENTRY_NUM), SECTOR_SIZE) + 1)
>  
> +/*
> + * attribute ioctls, same as their FAT equivalents.
> + */
> +#define EXFAT_IOCTL_GET_ATTRIBUTES	_IOR('r', 0x10, __u32)
> +#define EXFAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u32)

Can you reuse the definitions from include/uapi/linux/msdos_fs.h instead
of redefining them here?

Otherwise this looks like a mostly straight port of the fs/fat/
versions of these functions.

--D

> +
>  struct exfat_dentry_namebuf {
>  	char *lfn;
>  	int lfnbuf_len; /* usually MAX_UNINAME_BUF_SIZE */
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index 3cbd270e0cba..b31ce0868ddd 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -8,6 +8,8 @@
>  #include <linux/cred.h>
>  #include <linux/buffer_head.h>
>  #include <linux/blkdev.h>
> +#include <linux/fsnotify.h>
> +#include <linux/security.h>
>  
>  #include "exfat_raw.h"
>  #include "exfat_fs.h"
> @@ -316,6 +318,92 @@ int exfat_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  	return error;
>  }
>  
> +/*
> + * modified ioctls from fat/file.c by Welmer Almesberger
> + */
> +static int exfat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
> +{
> +	u32 attr;
> +
> +	inode_lock_shared(inode);
> +	attr = exfat_make_attr(inode);
> +	inode_unlock_shared(inode);
> +
> +	return put_user(attr, user_attr);
> +}
> +
> +static int exfat_ioctl_set_attributes(struct file *file, u32 __user *user_attr)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct exfat_sb_info *sbi = EXFAT_SB(inode->i_sb);
> +	int is_dir = S_ISDIR(inode->i_mode);
> +	u32 attr, oldattr;
> +	struct iattr ia;
> +	int err;
> +
> +	err = get_user(attr, user_attr);
> +	if (err)
> +		goto out;
> +
> +	err = mnt_want_write_file(file);
> +	if (err)
> +		goto out;
> +	inode_lock(inode);
> +
> +	oldattr = exfat_make_attr(inode);
> +
> +	/*
> +	 * Mask attributes so we don't set reserved fields.
> +	 */
> +	attr &= (ATTR_READONLY | ATTR_HIDDEN | ATTR_SYSTEM | ATTR_ARCHIVE);
> +	attr |= (is_dir ? ATTR_SUBDIR : 0);
> +
> +	/* Equivalent to a chmod() */
> +	ia.ia_valid = ATTR_MODE | ATTR_CTIME;
> +	ia.ia_ctime = current_time(inode);
> +	if (is_dir)
> +		ia.ia_mode = exfat_make_mode(sbi, attr, 0777);
> +	else
> +		ia.ia_mode = exfat_make_mode(sbi, attr, 0666 | (inode->i_mode & 0111));
> +
> +	/* The root directory has no attributes */
> +	if (inode->i_ino == EXFAT_ROOT_INO && attr != ATTR_SUBDIR) {
> +		err = -EINVAL;
> +		goto out_unlock_inode;
> +	}
> +
> +	if (((attr | oldattr) & ATTR_SYSTEM) &&
> +	    !capable(CAP_LINUX_IMMUTABLE)) {
> +		err = -EPERM;
> +		goto out_unlock_inode;
> +	}
> +
> +	/*
> +	 * The security check is questionable...  We single
> +	 * out the RO attribute for checking by the security
> +	 * module, just because it maps to a file mode.
> +	 */
> +	err = security_inode_setattr(file_mnt_idmap(file),
> +				     file->f_path.dentry, &ia);
> +	if (err)
> +		goto out_unlock_inode;
> +
> +	/* This MUST be done before doing anything irreversible... */
> +	err = exfat_setattr(file_mnt_idmap(file), file->f_path.dentry, &ia);
> +	if (err)
> +		goto out_unlock_inode;
> +
> +	fsnotify_change(file->f_path.dentry, ia.ia_valid);
> +
> +	exfat_save_attr(inode, attr);
> +	mark_inode_dirty(inode);
> +out_unlock_inode:
> +	inode_unlock(inode);
> +	mnt_drop_write_file(file);
> +out:
> +	return err;
> +}
> +
>  static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg)
>  {
>  	struct fstrim_range range;
> @@ -346,8 +434,13 @@ static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg)
>  long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	struct inode *inode = file_inode(filp);
> +	u32 __user *user_attr = (u32 __user *)arg;
>  
>  	switch (cmd) {
> +	case EXFAT_IOCTL_GET_ATTRIBUTES:
> +		return exfat_ioctl_get_attributes(inode, user_attr);
> +	case EXFAT_IOCTL_SET_ATTRIBUTES:
> +		return exfat_ioctl_set_attributes(filp, user_attr);
>  	case FITRIM:
>  		return exfat_ioctl_fitrim(inode, arg);
>  	default:
> -- 
> 2.40.1

Re: [PATCH v2] exfat: add ioctls for accessing attributes

[Jan Cincera]

On Mon, Aug 28, 2023, Darrick J. Wong wrote:
> On Sun, Aug 27, 2023 at 12:42:07PM +0200, Jan Cincera wrote:
>> Add GET and SET attributes ioctls to enable attribute modification.
>> We already do this in FAT and a few userspace utils made for it would
>> benefit from this also working on exFAT, namely fatattr.
>>
>> Signed-off-by: Jan Cincera <hcincera@gmail.com>
>> ---
>> Changes in v2:
>>   - Removed irrelevant comments.
>>   - Now masking reserved fields.
>>
>>  fs/exfat/exfat_fs.h |  6 +++
>>  fs/exfat/file.c     | 93 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 99 insertions(+)
>>
>> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
>> index 729ada9e26e8..ebe8c4b928f4 100644
>> --- a/fs/exfat/exfat_fs.h
>> +++ b/fs/exfat/exfat_fs.h
>> @@ -149,6 +149,12 @@ enum {
>>  #define DIR_CACHE_SIZE		\
>>  	(DIV_ROUND_UP(EXFAT_DEN_TO_B(ES_MAX_ENTRY_NUM), SECTOR_SIZE) + 1)
>>  
>> +/*
>> + * attribute ioctls, same as their FAT equivalents.
>> + */
>> +#define EXFAT_IOCTL_GET_ATTRIBUTES	_IOR('r', 0x10, __u32)
>> +#define EXFAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u32)
> 
> Can you reuse the definitions from include/uapi/linux/msdos_fs.h instead
> of redefining them here?
> 
> Otherwise this looks like a mostly straight port of the fs/fat/
> versions of these functions.
> 
> --D
> 

I don't think that's necessary right now, as exfat doesn't reuse attribute
definitions either. With some refactoring of existing code we could reuse
both, but I've been trying to keep this patch as simple as possible.

Jan Cincera

Re: [PATCH v2] exfat: add ioctls for accessing attributes

[Namjae Jeon]

2023-08-27 19:42 GMT+09:00, Jan Cincera <hcincera@gmail.com>:
> Add GET and SET attributes ioctls to enable attribute modification.
> We already do this in FAT and a few userspace utils made for it would
> benefit from this also working on exFAT, namely fatattr.
>
> Signed-off-by: Jan Cincera <hcincera@gmail.com>
Applied, Thanks for your patch!