[PATCH v6 0/3] fs: fat: add ioctl to modify fat filesystem partion volume label

[PATCH v6 0/3] fs: fat: add ioctl to modify fat filesystem partion volume label

[ChenGuanqiao]

The FAT filesystem partition volume label can be read with
FAT_IOCTL_GET_VOLUME_LABEL and written with FAT_IOCTL_SET_VOLUME_LABEL.

FAT volume label (volume name) is exactly same stored in boot sector and root
directory. Thus, the boot sector just needs to be upgrade when the label
writing.

v6:
1. fixed volume label name check function.
2. add volume label entry if no volume label.
v5:
1. find the volume label entry through the scan function.
2. the volume label only retains the d-characters (reference from Ecma-107).
v4:
1. read/write volume label from/to the location of the respective version.
2. correct volume label check reference from mkfs.fat.
3. fixed some code issue.
v3:
1. write volume label both boot sector and root directory.
v2:
1. add filesystem version check.
2. add diretory permissions check.
3. add volume label string check.
4. fixed part of return value.
5. fixed some indent issue.
6. remove sync_dirty_buffer().

ChenGuanqiao (3):
  fs: fat: Add fat filesystem partition volume label in local structure
  fs: fat: Add volume label entry method function
  fs: fat: add ioctl method in fat filesystem driver

 fs/fat/dir.c                  |  62 +++++++++++++++++
 fs/fat/fat.h                  |   6 ++
 fs/fat/file.c                 | 150 ++++++++++++++++++++++++++++++++++++++++++
 fs/fat/inode.c                |  15 ++++-
 include/uapi/linux/msdos_fs.h |   2 +
 5 files changed, 232 insertions(+), 3 deletions(-)

--
2.11.0

[PATCH v6 1/3] fs: fat: Add fat filesystem partition volume label in local structure

[ChenGuanqiao]

Signed-off-by: ChenGuanqiao <chen.chenchacha@foxmail.com>
---
 fs/fat/fat.h                  |  6 ++++++
 fs/fat/inode.c                | 15 ++++++++++++---
 include/uapi/linux/msdos_fs.h |  2 ++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 051dac1ce3be..0b7b635742fc 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -85,6 +85,7 @@ struct msdos_sb_info {
	int dir_per_block;	      /* dir entries per block */
	int dir_per_block_bits;	      /* log2(dir_per_block) */
	unsigned int vol_id;		/*volume ID*/
+	char vol_label[11];		/*volume label*/

	int fatent_shift;
	const struct fatent_operations *fatent_ops;
@@ -299,6 +300,11 @@ extern int fat_dir_empty(struct inode *dir);
 extern int fat_subdirs(struct inode *dir);
 extern int fat_scan(struct inode *dir, const unsigned char *name,
		    struct fat_slot_info *sinfo);
+extern int fat_scan_volume_label(struct inode *dir, struct buffer_head **bh,
+				 struct msdos_dir_entry **de);
+extern int fat_add_volume_label_entry(struct inode*dir,
+				      const unsigned char *name,
+				      struct timespec *ts);
 extern int fat_scan_logstart(struct inode *dir, int i_logstart,
			     struct fat_slot_info *sinfo);
 extern int fat_get_dotdot_entry(struct inode *dir, struct buffer_head **bh,
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 30c52394a7ad..e73379a41d49 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -45,12 +45,14 @@ struct fat_bios_param_block {

	u8	fat16_state;
	u32	fat16_vol_id;
+	u8	fat16_vol_label[11];

	u32	fat32_length;
	u32	fat32_root_cluster;
	u16	fat32_info_sector;
	u8	fat32_state;
	u32	fat32_vol_id;
+	u8	fat32_vol_label[11];
 };

 static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
@@ -1460,12 +1462,16 @@ static int fat_read_bpb(struct super_block *sb, struct fat_boot_sector *b,

	bpb->fat16_state = b->fat16.state;
	bpb->fat16_vol_id = get_unaligned_le32(b->fat16.vol_id);
+	memcpy(bpb->fat16_vol_label, b->fat16.vol_label,
+		   sizeof(bpb->fat16_vol_label));

	bpb->fat32_length = le32_to_cpu(b->fat32.length);
	bpb->fat32_root_cluster = le32_to_cpu(b->fat32.root_cluster);
	bpb->fat32_info_sector = le16_to_cpu(b->fat32.info_sector);
	bpb->fat32_state = b->fat32.state;
	bpb->fat32_vol_id = get_unaligned_le32(b->fat32.vol_id);
+	memcpy(bpb->fat32_vol_label, b->fat32.vol_label,
+		   sizeof(bpb->fat32_vol_label));

	/* Validate this looks like a FAT filesystem BPB */
	if (!bpb->fat_reserved) {
@@ -1723,11 +1729,14 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
		brelse(fsinfo_bh);
	}

-	/* interpret volume ID as a little endian 32 bit integer */
-	if (sbi->fat_bits == 32)
+	/* interpret volume ID and label as a little endian 32 bit integer */
+	if (sbi->fat_bits == 32) {
		sbi->vol_id = bpb.fat32_vol_id;
-	else /* fat 16 or 12 */
+		memcpy(sbi->vol_label, bpb.fat32_vol_label, sizeof(sbi->vol_label));
+	} else { /* fat 16 or 12 */
		sbi->vol_id = bpb.fat16_vol_id;
+		memcpy(sbi->vol_label, bpb.fat16_vol_label, sizeof(sbi->vol_label));
+	}

	sbi->dir_per_block = sb->s_blocksize / sizeof(struct msdos_dir_entry);
	sbi->dir_per_block_bits = ffs(sbi->dir_per_block) - 1;
diff --git a/include/uapi/linux/msdos_fs.h b/include/uapi/linux/msdos_fs.h
index e956704f5fb1..b4fced0988a9 100644
--- a/include/uapi/linux/msdos_fs.h
+++ b/include/uapi/linux/msdos_fs.h
@@ -106,6 +106,8 @@ struct __fat_dirent {
 #define FAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u32)
 /*Android kernel has used 0x12, so we use 0x13*/
 #define FAT_IOCTL_GET_VOLUME_ID		_IOR('r', 0x13, __u32)
+#define FAT_IOCTL_GET_VOLUME_LABEL	_IOR('r', 0x14, __u8[11])
+#define FAT_IOCTL_SET_VOLUME_LABEL	_IOW('r', 0x15, __u8[11])

 struct fat_boot_sector {
	__u8	ignored[3];	/* Boot strap short or near jump */
--
2.11.0

[PATCH v6 2/3] fs: fat: Add volume label entry method function

[ChenGuanqiao]

Signed-off-by: ChenGuanqiao <chen.chenchacha@foxmail.com>
---
 fs/fat/dir.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 81cecbe6d7cf..63aab656c08e 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -881,6 +881,68 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
	return -ENOENT;
 }

+static int fat_get_volume_label_entry(struct inode *dir, loff_t *pos,
+			       struct buffer_head **bh,
+			       struct msdos_dir_entry **de)
+{
+	while (fat_get_entry(dir, pos, bh, de) >= 0) {
+		if (((*de)->attr & ATTR_VOLUME) && (*de)->attr != ATTR_EXT)
+			return 0;
+	}
+	return -ENOENT;
+}
+
+int fat_scan_volume_label(struct inode *dir, struct buffer_head **bh,
+			  struct msdos_dir_entry **de)
+{
+	loff_t offset = 0;
+
+	if (dir->i_ino != MSDOS_ROOT_INO)
+		return -EINVAL;
+
+	*bh = NULL;
+	*de = NULL;
+	while (fat_get_volume_label_entry(dir, &offset, bh, de) >= 0)
+		return 0;
+
+	return -ENOENT;
+}
+EXPORT_SYMBOL_GPL(fat_scan_volume_label);
+
+int fat_add_volume_label_entry(struct inode *dir, const unsigned char *name,
+			       struct timespec *ts)
+{
+	struct msdos_sb_info *sbi = MSDOS_SB(dir->i_sb);
+	struct msdos_dir_entry de;
+	struct fat_slot_info sinfo;
+	__le16 time, date;
+	int err;
+
+	if (dir->i_ino != MSDOS_ROOT_INO)
+		return -EINVAL;
+
+	memcpy(de.name, name, MSDOS_NAME);
+	de.attr = ATTR_VOLUME;
+	de.lcase = 0;
+	fat_time_unix2fat(sbi, ts, &time, &date, NULL);
+	de.cdate = de.adate = 0;
+	de.ctime = 0;
+	de.ctime_cs = 0;
+	de.time = time;
+	de.date = date;
+	fat_set_start(&de, 0);
+	de.size = 0;
+
+	err = fat_add_entries(dir, &de, 1, &sinfo);
+	if (err)
+		return err;
+
+	dir->i_ctime = dir->i_mtime = *ts;
+
+	return fat_sync_inode(dir);
+}
+EXPORT_SYMBOL_GPL(fat_add_volume_label_entry);
+
 /*
  * The ".." entry can not provide the "struct fat_slot_info" information
  * for inode, nor a usable i_pos. So, this function provides some information
--
2.11.0

Re: [PATCH v6 2/3] fs: fat: Add volume label entry method function

[OGAWA Hirofumi]

ChenGuanqiao <chen.chenchacha@foxmail.com> writes:

> +static int fat_get_volume_label_entry(struct inode *dir, loff_t *pos,
> +			       struct buffer_head **bh,
> +			       struct msdos_dir_entry **de)
> +{
> +	while (fat_get_entry(dir, pos, bh, de) >= 0) {
> +		if (((*de)->attr & ATTR_VOLUME) && (*de)->attr != ATTR_EXT)

It should check IS_FREE().

> +			return 0;
> +	}
> +	return -ENOENT;
> +}
> +
> +int fat_scan_volume_label(struct inode *dir, struct buffer_head **bh,
> +			  struct msdos_dir_entry **de)
> +{
> +	loff_t offset = 0;
> +
> +	if (dir->i_ino != MSDOS_ROOT_INO)
> +		return -EINVAL;
> +
> +	*bh = NULL;
> +	*de = NULL;
> +	while (fat_get_volume_label_entry(dir, &offset, bh, de) >= 0)
> +		return 0;
> +
> +	return -ENOENT;
> +}
> +EXPORT_SYMBOL_GPL(fat_scan_volume_label);

No need this wrapper? Looks like better to merge into
fat_get_volume_label_entry().

> +int fat_add_volume_label_entry(struct inode *dir, const unsigned char *name,
> +			       struct timespec *ts)
> +{
> +	struct msdos_sb_info *sbi = MSDOS_SB(dir->i_sb);
> +	struct msdos_dir_entry de;
> +	struct fat_slot_info sinfo;
> +	__le16 time, date;
> +	int err;
> +
> +	if (dir->i_ino != MSDOS_ROOT_INO)
> +		return -EINVAL;
> +
> +	memcpy(de.name, name, MSDOS_NAME);
> +	de.attr = ATTR_VOLUME;
> +	de.lcase = 0;
> +	fat_time_unix2fat(sbi, ts, &time, &date, NULL);
> +	de.cdate = de.adate = 0;
> +	de.ctime = 0;
> +	de.ctime_cs = 0;
> +	de.time = time;
> +	de.date = date;
> +	fat_set_start(&de, 0);
> +	de.size = 0;
> +
> +	err = fat_add_entries(dir, &de, 1, &sinfo);
> +	if (err)
> +		return err;

Missing call of brelse(sinfo.bh)

> +	dir->i_ctime = dir->i_mtime = *ts;

Probably, we should not update this for labal.

> +	return fat_sync_inode(dir);

No inode for root.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCH v6 3/3] fs: fat: add ioctl method in fat filesystem driver

[ChenGuanqiao]

Signed-off-by: ChenGuanqiao <chen.chenchacha@foxmail.com>
---
 fs/fat/file.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4724cc9ad650..829c91ef1ea0 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -15,11 +15,36 @@
 #include <linux/fsnotify.h>
 #include <linux/security.h>
 #include <linux/falloc.h>
+#include <linux/ctype.h>
 #include "fat.h"

 static long fat_fallocate(struct file *file, int mode,
			  loff_t offset, loff_t len);

+static int fat_check_d_characters(char *label, unsigned long len)
+{
+	int i;
+
+	for (i=0; i<len; ++i) {
+		switch (label[i]) {
+		case '0' ... '9':
+		case 'A' ... 'Z':
+		case '_':
+			continue;
+		case 0x20:
+			return 0;
+		default:
+			break;
+		}
+		break;
+	}
+
+	if (i == len)
+		return 0;
+	else
+		return -EINVAL;
+}
+
 static int fat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
 {
	u32 attr;
@@ -121,10 +146,131 @@ static int fat_ioctl_get_volume_id(struct inode *inode, u32 __user *user_attr)
	return put_user(sbi->vol_id, user_attr);
 }

+static int fat_ioctl_get_volume_label(struct inode *inode,
+				      u8 __user *vol_label)
+{
+	int err = 0;
+	struct buffer_head *bh;
+	struct msdos_dir_entry *de;
+
+	inode_lock_shared(inode);
+	err = fat_scan_volume_label(inode, &bh, &de);
+	if (err)
+		goto out;
+
+	if (copy_to_user(vol_label, de->name, MSDOS_NAME))
+		err = -EFAULT;
+
+	brelse(bh);
+out:
+	inode_unlock_shared(inode);
+
+	return err;
+}
+
+static int fat_ioctl_set_volume_label(struct file *file,
+				      u8 __user *vol_label)
+{
+	int err = 0;
+	u8 label[MSDOS_NAME];
+	struct buffer_head *boot_bh;
+	struct buffer_head *vol_bh;
+	struct msdos_dir_entry *de;
+	struct fat_boot_sector *b;
+	struct inode *inode = file_inode(file);
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+
+	if (inode->i_ino != MSDOS_ROOT_INO) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (copy_from_user(label, vol_label, sizeof(label))) {
+		err = -EFAULT;
+		goto out;
+	}
+
+	err = fat_check_d_characters(label, sizeof(label));
+	if (err)
+		goto out;
+
+	err = mnt_want_write_file(file);
+	if (err)
+		goto out;
+
+	down_write(&sb->s_umount);
+	inode_lock(inode);
+
+	/* Updates root directory's vol_label */
+	err = fat_scan_volume_label(inode, &vol_bh, &de);
+	if (err) {
+		/* Create volume label entry */
+		struct timespec ts;
+
+		mutex_lock(&sbi->s_lock);
+		ts = current_time(inode);
+		err = fat_add_volume_label_entry(inode, label, &ts);
+		mutex_unlock(&sbi->s_lock);
+
+		if (err)
+			goto out_vol_brelse;
+	} else {
+		/* Write to root directory */
+		lock_buffer(vol_bh);
+		memcpy(de->name, label, sizeof(de->name));
+		mark_buffer_dirty(vol_bh);
+		unlock_buffer(vol_bh);
+	}
+
+	/* Update sector's vol_label */
+	boot_bh = sb_bread(sb, 0);
+	if (boot_bh == NULL) {
+		fat_msg(sb, KERN_ERR,
+			"unable to read boot sector to write volume label");
+		err = -EIO;
+		goto out_boot_brelse;
+	}
+
+	b = (struct fat_boot_sector *)boot_bh->b_data;
+	lock_buffer(boot_bh);
+	if (sbi->fat_bits == 32)
+		memcpy(b->fat32.vol_label, label, sizeof(label));
+	else
+		memcpy(b->fat16.vol_label, label, sizeof(label));
+
+	mark_buffer_dirty(boot_bh);
+	unlock_buffer(boot_bh);
+
+	/* Synchronize the data together */
+	err = sync_dirty_buffer(boot_bh);
+	if (err)
+		goto out_boot_brelse;
+
+	err = sync_dirty_buffer(vol_bh);
+	if (err)
+		goto out_boot_brelse;
+
+	/* Flush sbi */
+	memcpy(sbi->vol_label, label, sizeof(sbi->vol_label));
+
+out_boot_brelse:
+	brelse(boot_bh);
+out_vol_brelse:
+	brelse(vol_bh);
+
+	inode_unlock(inode);
+	up_write(&sb->s_umount);
+	mnt_drop_write_file(file);
+out:
+	return err;
+}
+
 long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
	struct inode *inode = file_inode(filp);
	u32 __user *user_attr = (u32 __user *)arg;
+	u8 __user *user_vol_label = (u8 __user *)arg;

	switch (cmd) {
	case FAT_IOCTL_GET_ATTRIBUTES:
@@ -133,6 +279,10 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
		return fat_ioctl_set_attributes(filp, user_attr);
	case FAT_IOCTL_GET_VOLUME_ID:
		return fat_ioctl_get_volume_id(inode, user_attr);
+	case FAT_IOCTL_GET_VOLUME_LABEL:
+		return fat_ioctl_get_volume_label(inode, user_vol_label);
+	case FAT_IOCTL_SET_VOLUME_LABEL:
+		return fat_ioctl_set_volume_label(filp, user_vol_label);
	default:
		return -ENOTTY;	/* Inappropriate ioctl for device */
	}
--
2.11.0

Re: [PATCH v6 3/3] fs: fat: add ioctl method in fat filesystem driver

[OGAWA Hirofumi]

ChenGuanqiao <chen.chenchacha@foxmail.com> writes:

> +static int fat_check_d_characters(char *label, unsigned long len)
> +{
> +	int i;
> +
> +	for (i=0; i<len; ++i) {

coding style. "i=0" to "i = 0", etc.

> +		switch (label[i]) {
> +		case '0' ... '9':
> +		case 'A' ... 'Z':
> +		case '_':
> +			continue;
> +		case 0x20:
> +			return 0;

Hm, stop check at ' '? What if "aaa b.%%%"?

> +static int fat_ioctl_get_volume_label(struct inode *inode,
> +				      u8 __user *vol_label)
> +{
> +	int err = 0;
> +	struct buffer_head *bh;
> +	struct msdos_dir_entry *de;

Hm, user has to care to open rootdir for this ioctl? Isn't it better to
force use the root inode?

> +	inode_lock_shared(inode);
> +	err = fat_scan_volume_label(inode, &bh, &de);
> +	if (err)
> +		goto out;
> +
> +	if (copy_to_user(vol_label, de->name, MSDOS_NAME))
> +		err = -EFAULT;

Better to copy to user buffer outside locking.

> +static int fat_ioctl_set_volume_label(struct file *file,
> +				      u8 __user *vol_label)
> +{
> +	int err = 0;
> +	u8 label[MSDOS_NAME];
> +	struct buffer_head *boot_bh;
> +	struct buffer_head *vol_bh;
> +	struct msdos_dir_entry *de;
> +	struct fat_boot_sector *b;
> +	struct inode *inode = file_inode(file);
> +	struct super_block *sb = inode->i_sb;
> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +
> +	if (inode->i_ino != MSDOS_ROOT_INO) {

Same with above, better to force use the root inode.

> +	down_write(&sb->s_umount);
> +	inode_lock(inode);
> +
> +	/* Updates root directory's vol_label */
> +	err = fat_scan_volume_label(inode, &vol_bh, &de);
> +	if (err) {
> +		/* Create volume label entry */
> +		struct timespec ts;
> +
> +		mutex_lock(&sbi->s_lock);

No need sbi->s_lock?

> +		ts = current_time(inode);
> +		err = fat_add_volume_label_entry(inode, label, &ts);
> +		mutex_unlock(&sbi->s_lock);
> +
> +		if (err)
> +			goto out_vol_brelse;
> +	} else {
> +		/* Write to root directory */
> +		lock_buffer(vol_bh);

No need lock_buffer()?

> +		memcpy(de->name, label, sizeof(de->name));

Probably, update timestamp?

> +		mark_buffer_dirty(vol_bh);
> +		unlock_buffer(vol_bh);
> +	}
> +
> +	/* Update sector's vol_label */
> +	boot_bh = sb_bread(sb, 0);
> +	if (boot_bh == NULL) {
> +		fat_msg(sb, KERN_ERR,
> +			"unable to read boot sector to write volume label");
> +		err = -EIO;
> +		goto out_boot_brelse;
> +	}
> +
> +	b = (struct fat_boot_sector *)boot_bh->b_data;
> +	lock_buffer(boot_bh);

No need lock_buffer()?

> +	if (sbi->fat_bits == 32)
> +		memcpy(b->fat32.vol_label, label, sizeof(label));
> +	else
> +		memcpy(b->fat16.vol_label, label, sizeof(label));
> +
> +	mark_buffer_dirty(boot_bh);
> +	unlock_buffer(boot_bh);
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>