| 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
| ||
|
Message-ID:
<PUZPR04MB63160C89856D1164322B643E8106A@PUZPR04MB6316.apcprd04.prod.outlook.com>
Date: Tue, 2 Sep 2025 04:55:58 +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: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/1] exfat: Add support for FS_IOC_{GET,SET}FSLABEL
Hi,
I have 3 more comments.
> 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 | 5 +
> fs/exfat/exfat_raw.h | 6 ++
> fs/exfat/file.c | 88 +++++++++++++++++
> fs/exfat/super.c | 224 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 323 insertions(+)
>
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> index f8ead4d47ef0..ed4b5ecb952b 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; /* volume name */
>
> unsigned int clu_srch_ptr; /* cluster search pointer */
> unsigned int used_clusters; /* number of used clusters */
> @@ -431,6 +432,10 @@ static inline loff_t exfat_ondisk_size(const struct
> inode *inode)
[snip]
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index 538d2b6ac2ec..970e3ee57c43 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,10 +487,93 @@ 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;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + uniname = kmalloc(sizeof(struct exfat_uni_name), GFP_KERNEL);
> + if (!uniname)
> + return -ENOMEM;
> +
> + ret = exfat_read_volume_label(sb);
> + if (ret < 0)
> + goto cleanup;
> +
> + memcpy(uniname->name, sbi->volume_label,
> + EXFAT_VOLUME_LABEL_LEN * sizeof(short));
> + uniname->name[EXFAT_VOLUME_LABEL_LEN] = 0x0000;
> + uniname->name_len = UniStrnlen(uniname->name,
> EXFAT_VOLUME_LABEL_LEN);
> +
> + 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,
> + struct inode *root_inode)
> +{
> + 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;
> + }
> +
> + ret = exfat_write_volume_label(sb, uniname, root_inode);
> +
> +cleanup:
> + kfree(uniname);
> + return ret;
> +}
> +
> 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;
> + struct inode *root_inode = filp->f_path.mnt->mnt_root->d_inode;
>
> switch (cmd) {
> case FAT_IOCTL_GET_ATTRIBUTES:
> @@ -500,6 +584,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,
> root_inode);
> default:
> return -ENOTTY;
> }
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 8926e63f5bb7..7931cdb4a1d1 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,228 @@ 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,
> + struct inode *root_inode)
> +{
> + int i, ret;
> + unsigned int type, old_clu;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + struct exfat_chain clu;
> + struct exfat_dentry *ep, *deleted_ep = NULL;
> + struct buffer_head *bh, *deleted_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) {
> + ret = -EIO;
> + goto end;
> + }
> +
> + type = exfat_get_entry_type(ep);
> + if (type == TYPE_DELETED && !deleted_ep && root_inode)
> {
> + deleted_ep = ep;
> + deleted_bh = bh;
> + continue;
> + }
> +
> + if (type == TYPE_UNUSED) {
> + if (!root_inode) {
> + brelse(bh);
> + ret = -ENOENT;
> + goto end;
> + }
> +
> + if (deleted_ep) {
> + brelse(bh);
> + goto end;
> + }
> +
> + if (i < sbi->dentries_per_clu - 1) {
> + deleted_ep = ep;
> + deleted_bh = bh;
> +
> + ep = exfat_get_dentry(sb, &clu,
> + i + 1, &bh);
> + memset(ep, 0,
> + sizeof(struct exfat_dentry));
> + ep->type = EXFAT_UNUSED;
> + exfat_update_bh(bh, true);
> + brelse(bh);
> +
> + goto end;
> + }
> +
> + // Last dentry in cluster
Please use /* */ to comment.
> + clu.size = 0;
> + old_clu = clu.dir;
> + ret = exfat_alloc_cluster(root_inode, 1,
> + &clu, true);
> + if (ret < 0) {
> + brelse(bh);
> + goto end;
> + }
In exFAT, directory size is limited to 256MB. Please add a check to return -ENOSPC
instead of allocating a new cluster if the root directory size had reached this limit.
> +
> + ret = exfat_ent_set(sb, old_clu, clu.dir);
> + if (ret < 0) {
> + exfat_free_cluster(root_inode, &clu);
> + brelse(bh);
> + goto end;
> + }
> +
> + ret = exfat_zeroed_cluster(root_inode, clu.dir);
> + if (ret < 0) {
> + exfat_free_cluster(root_inode, &clu);
> + brelse(bh);
> + goto end;
> + }
After allocating a new cluster for the root directory, its size needs to be updated.
> +
> + deleted_ep = ep;
> + deleted_bh = bh;
> + goto end;
> + }
> +
> + if (type == TYPE_VOLUME) {
> + *out_bh = bh;
> + *out_dentry = ep;
> +
> + if (deleted_ep)
> + brelse(deleted_bh);
> +
> + return 0;
> + }
> +
> + brelse(bh);
> + }
> +
> + if (exfat_get_next_cluster(sb, &(clu.dir))) {
> + ret = -EIO;
> + goto end;
> + }
> + }
> +
> + ret = -EIO;
> +
> +end:
> + if (deleted_ep) {
> + *out_bh = deleted_bh;
> + *out_dentry = deleted_ep;
> + memset((*out_dentry), 0, sizeof(struct exfat_dentry));
> + (*out_dentry)->type = EXFAT_VOLUME;
> + return 0;
> + }
> +
> + *out_bh = NULL;
> + *out_dentry = NULL;
> + return ret;
> +}
> +
> +static int exfat_alloc_volume_label(struct super_block *sb)
> +{
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + if (sbi->volume_label)
> + return 0;
> +
> +
> + mutex_lock(&sbi->s_lock);
> + sbi->volume_label = kcalloc(EXFAT_VOLUME_LABEL_LEN,
> + sizeof(short), GFP_KERNEL);
> + mutex_unlock(&sbi->s_lock);
> +
> + if (!sbi->volume_label)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +int exfat_read_volume_label(struct super_block *sb)
> +{
> + int ret, i;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + struct buffer_head *bh = NULL;
> + struct exfat_dentry *ep = NULL;
> +
> + ret = exfat_get_volume_label_ptrs(sb, &bh, &ep, NULL);
> + // ENOENT signifies that a volume label dentry doesn't exist
> + // We will treat this as an empty volume label and not fail.
> + if (ret < 0 && ret != -ENOENT)
> + goto cleanup;
> +
> + ret = exfat_alloc_volume_label(sb);
> + if (ret < 0)
> + goto cleanup;
> +
> + mutex_lock(&sbi->s_lock);
> + if (!ep)
> + memset(sbi->volume_label, 0, EXFAT_VOLUME_LABEL_LEN);
> + else
> + for (i = 0; i < EXFAT_VOLUME_LABEL_LEN; i++)
> + sbi->volume_label[i] = le16_to_cpu(ep-
> >dentry.volume_label.volume_label[i]);
> + mutex_unlock(&sbi->s_lock);
> +
> + ret = 0;
> +
> +cleanup:
> + if (bh)
> + brelse(bh);
> +
> + return ret;
> +}
> +
> +int exfat_write_volume_label(struct super_block *sb,
> + struct exfat_uni_name *uniname,
> + struct inode *root_inode)
> +{
> + int ret, i;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + struct buffer_head *bh = NULL;
> + struct exfat_dentry *ep = NULL;
> +
> + if (uniname->name_len > EXFAT_VOLUME_LABEL_LEN) {
> + ret = -EINVAL;
> + goto cleanup;
> + }
> +
> + ret = exfat_get_volume_label_ptrs(sb, &bh, &ep, root_inode);
> + if (ret < 0)
> + goto cleanup;
> +
> + ret = exfat_alloc_volume_label(sb);
> + if (ret < 0)
> + goto cleanup;
> +
> + memcpy(sbi->volume_label, uniname->name,
> + uniname->name_len * sizeof(short));
> +
> + mutex_lock(&sbi->s_lock);
> + for (i = 0; i < uniname->name_len; i++)
> + ep->dentry.volume_label.volume_label[i] =
> + cpu_to_le16(sbi->volume_label[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 = uniname->name_len;
> + mutex_unlock(&sbi->s_lock);
> +
> + ret = 0;
> +
> +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)
> @@ -791,6 +1014,7 @@ static void delayed_free(struct rcu_head *p)
>
> unload_nls(sbi->nls_io);
> exfat_free_upcase_table(sbi);
> + kfree(sbi->volume_label);
> exfat_free_sbi(sbi);
> }
>
> --
> 2.34.1
Powered by blists - more mailing lists