On Tue, Aug 19, 2025 at 10:22 PM Ethan Ferguson <ethan.ferguson@xxxxxxxxxx> wrote: > > On 8/18/25 21:45, Namjae Jeon wrote: > > On Sun, Aug 17, 2025 at 9:31 AM Ethan Ferguson > > <ethan.ferguson@xxxxxxxxxx> wrote: > >> > >> 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@xxxxxxxxxx> > >> > >> --- > >> fs/exfat/exfat_fs.h | 2 + > >> fs/exfat/exfat_raw.h | 6 +++ > >> fs/exfat/file.c | 56 +++++++++++++++++++++++++ > >> fs/exfat/super.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 163 insertions(+) > >> > >> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h > >> index f8ead4d47ef0..a764e6362172 100644 > >> --- a/fs/exfat/exfat_fs.h > >> +++ b/fs/exfat/exfat_fs.h > >> @@ -267,6 +267,7 @@ struct exfat_sb_info { > >> struct buffer_head **vol_amap; /* allocation bitmap */ > >> > >> unsigned short *vol_utbl; /* upcase table */ > >> + unsigned short volume_label[EXFAT_VOLUME_LABEL_LEN]; /* volume name */ > > There's no reason to have this in sbi. I think it's better to read the > > volume name in ioctl fslabel and return it. > > > That's fair. I wrote it this way because the volume label is stored in > the sbi in btrfs, but there it's (as far as I understand) part of the > fs header on disk, and not (as is the case in exfat) a directory entry > that could be arbitrarily far from the start of the disk. Maybe we could > cache it in the sbi after the first read? I'm open to either. I agree to cache it in sbi after the first read. > > >> > >> unsigned int clu_srch_ptr; /* cluster search pointer */ > >> unsigned int used_clusters; /* number of used clusters */ > >> @@ -431,6 +432,7 @@ 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_write_volume_label(struct super_block *sb); > >> > >> /* fatent.c */ > >> #define exfat_get_next_cluster(sb, pclu) exfat_ent_get(sb, *(pclu), pclu) > >> diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h > >> index 971a1ccd0e89..af04cef81c0c 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; > >> struct { > >> __u8 flags; > >> __u8 vendor_guid[16]; > >> diff --git a/fs/exfat/file.c b/fs/exfat/file.c > >> index 538d2b6ac2ec..c57d266aae3d 100644 > >> --- a/fs/exfat/file.c > >> +++ b/fs/exfat/file.c > >> @@ -12,6 +12,7 @@ > >> #include <linux/security.h> > >> #include <linux/msdos_fs.h> > >> #include <linux/writeback.h> > >> +#include "../nls/nls_ucs2_utils.h" > >> > >> #include "exfat_raw.h" > >> #include "exfat_fs.h" > >> @@ -486,6 +487,57 @@ 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_sb_info *sbi = EXFAT_SB(sb); > >> + size_t len = UniStrnlen(sbi->volume_label, EXFAT_VOLUME_LABEL_LEN); > >> + > >> + mutex_lock(&sbi->s_lock); > >> + ret = utf16s_to_utf8s(sbi->volume_label, len, > >> + UTF16_HOST_ENDIAN, utf8, FSLABEL_MAX); > >> + mutex_unlock(&sbi->s_lock); > >> + > >> + if (ret < 0) > >> + return ret; > >> + > >> + if (copy_to_user((char __user *)arg, utf8, FSLABEL_MAX)) > >> + return -EFAULT; > >> + > >> + return 0; > >> +} > >> + > >> +static int exfat_ioctl_set_volume_label(struct super_block *sb, unsigned long arg) > >> +{ > >> + int ret = 0; > >> + char utf8[FSLABEL_MAX]; > >> + size_t len; > >> + unsigned short utf16[EXFAT_VOLUME_LABEL_LEN] = {0}; > >> + struct exfat_sb_info *sbi = EXFAT_SB(sb); > >> + > >> + if (!capable(CAP_SYS_ADMIN)) > >> + return -EPERM; > >> + > >> + if (copy_from_user(utf8, (char __user *)arg, FSLABEL_MAX)) > >> + return -EFAULT; > >> + > >> + len = strnlen(utf8, FSLABEL_MAX); > >> + if (len > EXFAT_VOLUME_LABEL_LEN) > > Is FSLABEL_MAX in bytes or the number of characters ? > > > the definition mentions chars, and everywhere else it's used it's in > terms of chars, so I'd say it's in terms of bytes. The > FS_IOC_{GET,SET}FSLABEL ioctls are in terms of char[FSLABEL_MAX], so > I think it's reasonable to use it as a number of bytes. Okay. > > >> + exfat_info(sb, "Volume label length too long, truncating"); > >> + > >> + mutex_lock(&sbi->s_lock); > >> + ret = utf8s_to_utf16s(utf8, len, UTF16_HOST_ENDIAN, utf16, EXFAT_VOLUME_LABEL_LEN); > >> + mutex_unlock(&sbi->s_lock); > >> + > >> + if (ret < 0) > >> + return ret; > >> + > >> + memcpy(sbi->volume_label, utf16, sizeof(sbi->volume_label)); > >> + > >> + return exfat_write_volume_label(sb); > >> +} > >> + > >> long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > >> { > >> struct inode *inode = file_inode(filp); > >> @@ -500,6 +552,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/super.c b/fs/exfat/super.c > >> index 8926e63f5bb7..96cd4bb7cb19 100644 > >> --- a/fs/exfat/super.c > >> +++ b/fs/exfat/super.c > >> @@ -18,6 +18,7 @@ > >> #include <linux/nls.h> > >> #include <linux/buffer_head.h> > >> #include <linux/magic.h> > >> +#include "../nls/nls_ucs2_utils.h" > >> > >> #include "exfat_raw.h" > >> #include "exfat_fs.h" > >> @@ -573,6 +574,98 @@ 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) > >> +{ > >> + int i; > >> + unsigned int type; > >> + struct exfat_sb_info *sbi = EXFAT_SB(sb); > >> + struct exfat_chain clu; > >> + struct exfat_dentry *ep; > >> + struct buffer_head *bh; > >> + > >> + 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) > >> + return -EIO; > >> + > >> + type = exfat_get_entry_type(ep); > >> + if (type == TYPE_UNUSED) { > >> + brelse(bh); > >> + return -EIO; > >> + } > >> + > >> + if (type == TYPE_VOLUME) { > >> + *out_bh = bh; > >> + *out_dentry = ep; > >> + return 0; > >> + } > >> + > >> + brelse(bh); > >> + } > >> + > >> + if (exfat_get_next_cluster(sb, &(clu.dir))) > >> + return -EIO; > >> + } > >> + > >> + return -EIO; > >> +} > >> + > >> +static int exfat_read_volume_label(struct super_block *sb) > >> +{ > >> + int ret, i; > >> + struct exfat_sb_info *sbi = EXFAT_SB(sb); > >> + struct buffer_head *bh; > >> + struct exfat_dentry *ep; > >> + > >> + ret = exfat_get_volume_label_ptrs(sb, &bh, &ep); > >> + if (ret < 0) > >> + goto cleanup; > >> + > >> + for (i = 0; i < EXFAT_VOLUME_LABEL_LEN; i++) > >> + sbi->volume_label[i] = le16_to_cpu(ep->dentry.volume_label.volume_label[i]); > >> + > >> +cleanup: > >> + if (bh) > >> + brelse(bh); > >> + > >> + return ret; > >> +} > >> + > >> +int exfat_write_volume_label(struct super_block *sb) > >> +{ > >> + int ret, i; > >> + struct exfat_sb_info *sbi = EXFAT_SB(sb); > >> + struct buffer_head *bh; > >> + struct exfat_dentry *ep; > >> + > >> + ret = exfat_get_volume_label_ptrs(sb, &bh, &ep); > >> + if (ret < 0) > >> + goto cleanup; > >> + > >> + mutex_lock(&sbi->s_lock); > >> + for (i = 0; i < EXFAT_VOLUME_LABEL_LEN; i++) > >> + ep->dentry.volume_label.volume_label[i] = cpu_to_le16(sbi->volume_label[i]); > >> + > >> + ep->dentry.volume_label.char_count = > >> + UniStrnlen(sbi->volume_label, EXFAT_VOLUME_LABEL_LEN); > >> + mutex_unlock(&sbi->s_lock); > >> + > >> +cleanup: > >> + if (bh) { > >> + exfat_update_bh(bh, true); > >> + brelse(bh); > >> + } > >> + > >> + return ret; > >> +} > >> + > >> /* mount the file system volume */ > >> static int __exfat_fill_super(struct super_block *sb, > >> struct exfat_chain *root_clu) > >> @@ -616,6 +709,12 @@ static int __exfat_fill_super(struct super_block *sb, > >> goto free_bh; > >> } > >> > >> + ret = exfat_read_volume_label(sb); > > It will affect mount time if volume label entry is located at the end. > > So, we can read it in ioctl fslabel as I said above. > Sounds good. I'll incorporate your changes, and those of > yuezhang.mo@xxxxxxxx, and submit version 3 of the patch soon. As Yuezhang pointed out, You need to consider the case where there is no volume label entry. Some mkfs implementations, though not Windows, don't create a volume entry. So, if FS_IOC_SETFSLABEL doesn't find a volume label entry, it should either look for an empty entry, or, if that's not available, allocate a cluster to get a entry. Thanks for your work! > > >> + if (ret) { > >> + exfat_err(sb, "failed to read volume label"); > >> + goto free_bh; > >> + } > >> + > >> ret = exfat_count_used_clusters(sb, &sbi->used_clusters); > >> if (ret) { > >> exfat_err(sb, "failed to scan clusters"); > >> -- > >> 2.50.1 > >>