2023-08-24 13:48:48

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH] exfat: add ioctls for accessing attributes

2023-08-16 3:29 GMT+09:00, Jan Cincera <[email protected]>:
> 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 <[email protected]>
> ---
> fs/exfat/exfat_fs.h | 6 +++
> fs/exfat/file.c | 97 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 103 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..b358acbead27 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,96 @@ 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);
> +
> + /*
> + * ATTR_VOLUME and ATTR_SUBDIR cannot be changed; this also
> + * prevents the user from turning us into a VFAT
> + * longname entry. Also, we obviously can't set
> + * any of the NTFS attributes in the high 24 bits.
> + */
This comments seems to be wrong. In particular, The comment about
longfilename and ATTR_VOLUME for vfat should be removed.

> + attr &= 0xff & ~(ATTR_VOLUME | ATTR_SUBDIR);
> + /* Merge in ATTR_VOLUME and ATTR_DIR */
> + attr |= (EXFAT_I(inode)->attr & ATTR_VOLUME) |
> + (is_dir ? ATTR_SUBDIR : 0);
We need to mask exfat file attribute bits like the following not to
set reserved fields(+ ATTR_SUBDIR). And there is no ATTR_VOLUME field
in exfat.

Table 28 FileAttributes Field Structure
ReadOnly 0 1 This field is mandatory and conforms to the MS-DOS
definition.
Hidden 1 1 This field is mandatory and conforms to
the MS-DOS definition.
System 2 1 This field is mandatory and conforms to the
MS-DOS definition.
Reserved1 3 1 This field is mandatory and its contents are reserved.
Directory 4 1 This field is mandatory and conforms to the
MS-DOS definition.
Archive 5 1 This field is mandatory and conforms to the
MS-DOS definition.
Reserved2 6 10 This field is mandatory and its contents are reserved.

Thanks.