Skip to contentSkip to conversation

fs: fat: add ioctl to modify fat filesystem volume label

PATCH In review Chen Guanqiao 2 participants 2 messages Dec 20, 2017 VFAT/FAT/msdos filesystem

lore
Share & tools
lore

Chen Guanqiao

author
#0 lore raw

PATCH 1/1 fs: fat: add ioctl to modify fat filesystem volume label

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

Signed-off-by Chen Guanqiao <[email protected]>
---
 fs/fat/fat.h                  |  1 +
 fs/fat/file.c                 | 51 +++++++++++++++++++++++++++++++++++++++++++
 fs/fat/inode.c                | 15 ++++++++++---
 include/uapi/linux/msdos_fs.h |  2 ++
 4 files changed, 66 insertions(+), 3 deletions(-)
fs/fat/fat.h +0 -0
index 8fc1093da47d..204012c94421 100644
@@ -86,6 +86,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;
fs/fat/file.c +0 -0
index 4724cc9ad650..5d2990c44705 100644
@@ -121,6 +121,53 @@ 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,
+ u32 __user *user_attr)
+{
+ struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+ u8 __user *vol_label = (u8 __user *)user_attr;
+
+ return copy_to_user(vol_label, sbi->vol_label, sizeof(sbi->vol_label));
+}
+
+static int fat_ioctl_set_volume_label(struct inode *inode,
+ u32 __user *user_attr)
+{
+ struct buffer_head *bh;
+ struct fat_boot_sector *b;
+ struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+ u8 __user *vol_label = (u8 __user *)user_attr;
+ u8 label[11];
+
+ if (copy_from_user(label, vol_label, sizeof(label)))
+ return -EFAULT;
+
+ if (sb_rdonly(inode->i_sb))
+ return -EFAULT;
+
+ bh = sb_bread(inode->i_sb, 0);
+ if (bh == NULL) {
+ fat_msg(inode->i_sb, KERN_ERR,
+ "unable to read boot sector to write volume label");
+ return -EFAULT;
+ }
+
+ b = (struct fat_boot_sector *) bh->b_data;
+
+ 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(bh);
+ sync_dirty_buffer(bh);
+ brelse(bh);
+
+ memcpy(sbi->vol_label, label, sizeof(sbi->vol_label));
+
+ return 0;
+}
+
long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

 {
	struct inode *inode = file_inode(filp);
@@ -133,6 +180,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_attr);
+	case FAT_IOCTL_SET_VOLUME_LABEL:
+		return fat_ioctl_set_volume_label(inode, user_attr);
	default:
		return -ENOTTY;	/* Inappropriate ioctl for device */
	}
fs/fat/inode.c +0 -0
index 20a0a89eaca5..9991500c98af 100644
@@ -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;
include/uapi/linux/msdos_fs.h +2 -0
index a45d0754102e..799f9a6cb99d 100644
@@ -107,6 +107,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	_IOR('r', 0x15, __u8[11])
 
 struct fat_boot_sector {

__u8 ignored[3]; /* Boot strap short or near jump */
--
2.15.1

maintainer
#1 lore raw
Quoted 2 lines · Chen Guanqiao

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

Those vol_label should be matching with volume label in root directory, right? So I think handling only boot sector's vol_label would not work as expected.

Review

Quoted 2 lines
(quoted diff) +2 -0
+static int fat_ioctl_get_volume_label(struct inode *inode,
+									  u32 __user *user_attr)

Maybe you are using non-8 tab size, and so over 80 column.

Review

Quoted 3 lines
(quoted diff) +3 -0
+{
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	u8 __user *vol_label = (u8 __user *)user_attr;

This should not use strange cast (u32 => u8), instead caller should cast to proper one.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return copy_to_user(vol_label, sbi->vol_label, sizeof(sbi->vol_label));
+}

Returning result of copy_to_user() is strange. Probably, it should return 0 or -EFAULT.

Review

Quoted 2 lines
(quoted diff) +2 -0
+static int fat_ioctl_set_volume_label(struct inode *inode,
+									  u32 __user *user_attr)

same indent issue.

Review

Quoted 6 lines
(quoted diff) +6 -0
+{
+	struct buffer_head *bh;
+	struct fat_boot_sector *b;
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
+	u8 __user *vol_label = (u8 __user *)user_attr;
+	u8 label[11];

It should not allow to change for normal user that having only read access.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (copy_from_user(label, vol_label, sizeof(label)))
+		return -EFAULT;

It should check invalid label early (e.g. lower case chars, invalid chars, etc.).

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (sb_rdonly(inode->i_sb))
+		return -EFAULT;

-EROFS

Review

Quoted 4 lines
(quoted diff) +4 -0
+	bh = sb_bread(inode->i_sb, 0);
+	if (bh == NULL) {
+		fat_msg(inode->i_sb, KERN_ERR,
+				"unable to read boot sector to write volume label");

indent issue.

Review

Quoted 2 lines
(quoted diff) +2 -0
+		return -EFAULT;
+	}

It should take lock to prevent race.

Review

Quoted 9 lines
(quoted diff) +9 -0
+	b = (struct fat_boot_sector *) bh->b_data;
+
+	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(bh);
+	sync_dirty_buffer(bh);

It should check I/O error.

Quoted 2 lines

long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) {

	u8 __user *user_vol_label = (u8 __user *)arg;

Review

Quoted 4 lines
(quoted diff) +4 -0
+	case FAT_IOCTL_GET_VOLUME_LABEL:
+		return fat_ioctl_get_volume_label(inode, user_attr);
+	case FAT_IOCTL_SET_VOLUME_LABEL:
+		return fat_ioctl_set_volume_label(inode, user_attr);

s/user_attr/user_vol_label/

fs/fat/inode.c struct fat_bios_param_block { · line 50

Quoted 11 lines
fs/fat/inode.c +2 -0
index 20a0a89eaca5..9991500c98af 100644
@@ -45,12 +45,14 @@ struct fat_bios_param_block {
 
 	u8	fat16_state;
 	u32	fat16_vol_id;
+	u8  fat16_vol_label[11];
 
+	u8  fat32_vol_label[11];

indent.

fs/fat/inode.c struct fat_bios_param_block { · line 51

Quoted 1 line
fs/fat/inode.c +1 -0
+#define FAT_IOCTL_SET_VOLUME_LABEL	_IOR('r', 0x15, __u8[11])

_IOW

Thanks.

Signature · 1 line
OGAWA Hirofumi <[email protected]>