lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <PUZPR04MB63164B9367EBABF81B381D318103A@PUZPR04MB6316.apcprd04.prod.outlook.com>
Date: Fri, 5 Sep 2025 11:25:14 +0000
From: "Yuezhang.Mo@...y.com" <Yuezhang.Mo@...y.com>
To: Ethan Ferguson <ethan.ferguson@...ier.com>,
        "linkinjeon@...nel.org"
	<linkinjeon@...nel.org>,
        "sj1557.seo@...sung.com" <sj1557.seo@...sung.com>
CC: "cpgs@...sung.com" <cpgs@...sung.com>,
        "linux-fsdevel@...r.kernel.org"
	<linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 1/1] exfat: Add support for FS_IOC_{GET,SET}FSLABEL

> Add support for reading / writing to the exfat volume label from the
> FS_IOC_GETFSLABEL and FS_IOC_SETFSLABEL ioctls
> 
> Signed-off-by: Ethan Ferguson <ethan.ferguson@...ier.com>
> ---
>  fs/exfat/exfat_fs.h  |   7 ++
>  fs/exfat/exfat_raw.h |   6 ++
>  fs/exfat/file.c      |  80 +++++++++++++++++++++
>  fs/exfat/namei.c     |   2 +-
>  fs/exfat/super.c     | 165 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 259 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> index f8ead4d47ef0..a11a086c9d09 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -431,6 +431,10 @@ static inline loff_t exfat_ondisk_size(const struct inode *inode)
>  /* super.c */
>  int exfat_set_volume_dirty(struct super_block *sb);
>  int exfat_clear_volume_dirty(struct super_block *sb);
> +int exfat_read_volume_label(struct super_block *sb,
> +                           struct exfat_uni_name *label_out);
> +int exfat_write_volume_label(struct super_block *sb,
> +                            struct exfat_uni_name *label);
> 
>  /* fatent.c */
>  #define exfat_get_next_cluster(sb, pclu) exfat_ent_get(sb, *(pclu), pclu)
> @@ -477,6 +481,9 @@ int exfat_force_shutdown(struct super_block *sb, u32 flags);
>  /* namei.c */
>  extern const struct dentry_operations exfat_dentry_ops;
>  extern const struct dentry_operations exfat_utf8_dentry_ops;
> +int exfat_find_empty_entry(struct inode *inode,
> +               struct exfat_chain *p_dir, int num_entries,
> +                          struct exfat_entry_set_cache *es);
> 
>  /* cache.c */
>  int exfat_cache_init(void);
> diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
> index 971a1ccd0e89..4082fa7b8c14 100644
> --- a/fs/exfat/exfat_raw.h
> +++ b/fs/exfat/exfat_raw.h
> @@ -80,6 +80,7 @@
>  #define BOOTSEC_OLDBPB_LEN             53
> 
>  #define EXFAT_FILE_NAME_LEN            15
> +#define EXFAT_VOLUME_LABEL_LEN         11
> 
>  #define EXFAT_MIN_SECT_SIZE_BITS               9
>  #define EXFAT_MAX_SECT_SIZE_BITS               12
> @@ -159,6 +160,11 @@ struct exfat_dentry {
>                         __le32 start_clu;
>                         __le64 size;
>                 } __packed upcase; /* up-case table directory entry */
> +               struct {
> +                       __u8 char_count;
> +                       __le16 volume_label[EXFAT_VOLUME_LABEL_LEN];
> +                       __u8 reserved[8];
> +               } __packed volume_label; /* volume label directory entry */
>                 struct {
>                         __u8 flags;
>                         __u8 vendor_guid[16];
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index 538d2b6ac2ec..c44928c02474 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -486,6 +486,82 @@ static int exfat_ioctl_shutdown(struct super_block *sb, unsigned long arg)
>         return exfat_force_shutdown(sb, flags);
>  }
> 
> +static int exfat_ioctl_get_volume_label(struct super_block *sb, unsigned long arg)
> +{
> +       int ret;
> +       char utf8[FSLABEL_MAX] = {0};
> +       struct exfat_uni_name *uniname;
> +
> +       uniname = kmalloc(sizeof(struct exfat_uni_name), GFP_KERNEL);
> +       if (!uniname)
> +               return -ENOMEM;
> +
> +       ret = exfat_read_volume_label(sb, uniname);
> +       if (ret < 0)
> +               goto cleanup;
> +
> +       ret = exfat_utf16_to_nls(sb, uniname, utf8, FSLABEL_MAX);
> +       if (ret < 0)
> +               goto cleanup;
> +
> +       if (copy_to_user((char __user *)arg, utf8, FSLABEL_MAX)) {
> +               ret = -EFAULT;
> +               goto cleanup;
> +       }
> +
> +       ret = 0;
> +
> +cleanup:
> +       kfree(uniname);
> +       return ret;
> +}
> +
> +static int exfat_ioctl_set_volume_label(struct super_block *sb,
> +                                       unsigned long arg)
> +{
> +       int ret, lossy;
> +       char utf8[FSLABEL_MAX];
> +       struct exfat_uni_name *uniname;
> +
> +       if (!capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       uniname = kmalloc(sizeof(struct exfat_uni_name), GFP_KERNEL);
> +       if (!uniname)
> +               return -ENOMEM;
> +
> +       if (copy_from_user(utf8, (char __user *)arg, FSLABEL_MAX)) {
> +               ret = -EFAULT;
> +               goto cleanup;
> +       }
> +
> +       if (utf8[0]) {
> +               ret = exfat_nls_to_utf16(sb, utf8, strnlen(utf8, FSLABEL_MAX),
> +                                        uniname, &lossy);
> +               if (ret < 0) {
> +                       goto cleanup;
> +               } else if (lossy & NLS_NAME_LOSSY) {
> +                       ret = -EINVAL;
> +                       goto cleanup;
> +               }
> +       } else {
> +               uniname->name[0] = 0x0000;
> +               uniname->name_len = 0;
> +       }
> +
> +       if (uniname->name_len > EXFAT_VOLUME_LABEL_LEN) {
> +               exfat_info(sb, "Volume label length too long, truncating");
> +               uniname->name_len = EXFAT_VOLUME_LABEL_LEN;
> +               uniname->name[EXFAT_VOLUME_LABEL_LEN] = 0x0000;
> +       }
> +
> +       ret = exfat_write_volume_label(sb, uniname);
> +
> +cleanup:
> +       kfree(uniname);
> +       return ret;
> +}
> +
>  long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>         struct inode *inode = file_inode(filp);
> @@ -500,6 +576,10 @@ long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>                 return exfat_ioctl_shutdown(inode->i_sb, arg);
>         case FITRIM:
>                 return exfat_ioctl_fitrim(inode, arg);
> +       case FS_IOC_GETFSLABEL:
> +               return exfat_ioctl_get_volume_label(inode->i_sb, arg);
> +       case FS_IOC_SETFSLABEL:
> +               return exfat_ioctl_set_volume_label(inode->i_sb, arg);
>         default:
>                 return -ENOTTY;
>         }
> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
> index f5f1c4e8a29f..eaa781d6263c 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -300,7 +300,7 @@ static int exfat_check_max_dentries(struct inode *inode)
>   *   the directory entry index in p_dir is returned on succeeds
>   *   -error code is returned on failure
>   */
> -static int exfat_find_empty_entry(struct inode *inode,
> +int exfat_find_empty_entry(struct inode *inode,
>                 struct exfat_chain *p_dir, int num_entries,
>                 struct exfat_entry_set_cache *es)
>  {
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 8926e63f5bb7..0374e41b48a5 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -573,6 +573,171 @@ static int exfat_verify_boot_region(struct super_block *sb)
>         return 0;
>  }
> 
> +static int exfat_get_volume_label_ptrs(struct super_block *sb,
> +                                      struct buffer_head **out_bh,
> +                                      struct exfat_dentry **out_dentry,
> +                                      bool create)
> +{
> +       int i, ret;
> +       unsigned int type;
> +       struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +       struct inode *root_inode = sb->s_root->d_inode;
> +       struct exfat_inode_info *ei = EXFAT_I(root_inode);
> +       struct exfat_entry_set_cache es;
> +       struct exfat_chain clu;
> +       struct exfat_dentry *ep, *overwrite_ep = NULL;
> +       struct buffer_head *bh, *overwrite_bh = NULL;
> +
> +       clu.dir = sbi->root_dir;
> +       clu.flags = ALLOC_FAT_CHAIN;
> +
> +       while (clu.dir != EXFAT_EOF_CLUSTER) {
> +               for (i = 0; i < sbi->dentries_per_clu; i++) {
> +                       ep = exfat_get_dentry(sb, &clu, i, &bh);
> +
> +                       if (!ep) {
> +                               ret = -EIO;
> +                               goto error;
> +                       }
> +
> +                       type = exfat_get_entry_type(ep);
> +                       if ((type == TYPE_DELETED || type == TYPE_UNUSED)
> +                           && !overwrite_ep && create) {
> +                               overwrite_ep = ep;
> +                               overwrite_bh = bh;
> +                               continue;
> +                       }
> +
> +                       if (type == TYPE_VOLUME) {
> +                               *out_bh = bh;
> +                               *out_dentry = ep;
> +
> +                               brelse(overwrite_bh);
> +                               return 0;
> +                       }
> +
> +                       brelse(bh);
> +               }
> +
> +               if (exfat_get_next_cluster(sb, &(clu.dir))) {
> +                       ret = -EIO;
> +                       goto error;
> +               }
> +       }
> +
> +       if (!create) {
> +               ret = -ENOENT;
> +               goto error;
> +       }
> +
> +
> +       if (overwrite_ep) {
> +               ep = overwrite_ep;
> +               bh = overwrite_bh;
> +               goto overwrite;
> +       }
> +
> +       ret = exfat_find_empty_entry(root_inode, &clu, 1, &es);
> +       if (ret < 0)
> +               goto error;
> +
> +       ei->hint_femp.eidx = 0;
> +       ei->hint_femp.count = sbi->dentries_per_clu;
> +       ei->hint_femp.cur = clu;
> +
> +       ep = exfat_get_dentry_cached(&es, 0);
> +       bh = es.bh[EXFAT_B_TO_BLK(es.start_off, sb)];
> +       /* increment use counter so exfat_put_dentry_set doesn't free */
> +       get_bh(bh);
> +       ret = exfat_put_dentry_set(&es, false);
> +       if (ret < 0) {
> +               bforget(bh);
> +               goto error;
> +       }
> +       ei->hint_femp.eidx++;
> +       ei->hint_femp.count--;
> +
> +overwrite:
> +
> +       memset(ep, 0, sizeof(struct exfat_dentry));
> +       ep->type = EXFAT_VOLUME;
> +       *out_bh = bh;
> +       *out_dentry = ep;
> +       return 0;
> +
> +error:
> +       *out_bh = NULL;
> +       *out_dentry = NULL;
> +       return ret;
> +}

exfat_get_volume_label_ptrs() involves finding and allocating volume
label dentry, which makes the logic complicated.

I suggest defining a function that only find volume label dentry,
likes exfat_find_volume_label_dentry().

exfat_find_volume_label_dentry()
{
   while (clu.dir != EXFAT_EOF_CLUSTER) {
      for (i = 0; i < sbi->dentries_per_clu; i++, dentry++) {
          if (type == TYPE_DELETED || type == TYPE_UNUSED) {
                 if (hint_femp.eidx == EXFAT_HINT_NONE) {
                         set hint_femp
                 }

                 if (type == TYPE_UNUSED) {
                    not found
                 }
          }

          if (type == TYPE_VOLUME) {
             found
          }
      }
   }

   /* no empty entry, hint that empty entry is in the new cluster */
   if (hint_femp.eidx == EXFAT_HINT_NON) {
        set hint_femp
   }

   EXFAT_I(root_inode)->hint_femp = hint_femp;
}

> +
> +int exfat_read_volume_label(struct super_block *sb, struct exfat_uni_name *label_out)
> +{
> +       int ret, i;
> +       struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +       struct buffer_head *bh = NULL;
> +       struct exfat_dentry *ep = NULL;
> +
> +       mutex_lock(&sbi->s_lock);
> +
> +       ret = exfat_get_volume_label_ptrs(sb, &bh, &ep, false);
> +       // ENOENT signifies that a volume label dentry doesn't exist
> +       // We will treat this as an empty volume label and not fail.

Use /**/ for all comments.

> +       if (ret == -ENOENT) {
> +               label_out->name[0] = 0x0000;
> +               label_out->name_len = 0;
> +               ret = 0;
> +       } else if (ret < 0) {
> +               goto cleanup;
> +       } else {
> +               for (i = 0; i < EXFAT_VOLUME_LABEL_LEN; i++)
> +                       label_out->name[i] = le16_to_cpu(ep->dentry.volume_label.volume_label[i]);
> +               label_out->name_len = ep->dentry.volume_label.char_count;
> +       }
> +
> +cleanup:
> +       mutex_unlock(&sbi->s_lock);
> +       brelse(bh);
> +       return ret;
> +}
> +
> +int exfat_write_volume_label(struct super_block *sb,
> +                            struct exfat_uni_name *label)
> +{
> +       int ret, i;
> +       struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +       struct buffer_head *bh = NULL;
> +       struct exfat_dentry *ep = NULL;
> +
> +       if (label->name_len > EXFAT_VOLUME_LABEL_LEN)
> +               return -EINVAL;
> +
> +       mutex_lock(&sbi->s_lock);
> +
> +       ret = exfat_get_volume_label_ptrs(sb, &bh, &ep, true);
> +       if (ret < 0)
> +               goto cleanup;
> +
> +       for (i = 0; i < label->name_len; i++)
> +               ep->dentry.volume_label.volume_label[i] =
> +                       cpu_to_le16(label->name[i]);
> +       // Fill the rest of the str with 0x0000
> +       for (; i < EXFAT_VOLUME_LABEL_LEN; i++)
> +               ep->dentry.volume_label.volume_label[i] = 0x0000;
> +
> +       ep->dentry.volume_label.char_count = label->name_len;
> +
> +cleanup:
> +       mutex_unlock(&sbi->s_lock);
> +
> +       if (bh) {
> +               exfat_update_bh(bh, IS_DIRSYNC(sb->s_root->d_inode));
> +               brelse(bh);
> +       }
> +
> +       return ret;
> +}

If there is no volume label directory entry, no volume label
dentry needs to be allocated when clearing the volume label.

exfat_write_volume_label()
{
	exfat_find_volume_label_dentry();
	if (not found) {
		if (label->name_len == 0)
			/* return with nothing to do */

		exfat_find_empty_entry()
	}

	/* set or clear volume label */
}

In exfat, exfat_{read, write}_volume_label() are directory operations,
so it would be better to move them to dir.c.

> +
>  /* mount the file system volume */
>  static int __exfat_fill_super(struct super_block *sb,
>                 struct exfat_chain *root_clu)
> --
> 2.34.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ