Skip to contentSkip to conversation

fs: fat: add ioctl method to read voluem label in fat filesystem driver

PATCH In review chenchacha 2 participants 6 messages Oct 4, 2018 – Oct 6, 2018 2 messages not in the archive VFAT/FAT/msdos filesystem

lore
Share & tools
lore

chenchacha

author
#0 lore raw

cover fs: fat: add ioctl method to read voluem label in fat filesystem driver

This patch add fat ioctl command FAT_IOCTL_GET_VOLUME_LABEL.

We can get the volume label with this command:

	ioctl(fd, FAT_IOCTL_GET_VOLUME_LABEL, &volume_label);

FAT volume label (volume name) is exactly same stored in boot sector and root directory. And this patch read label only from the root directory. If label in root directory is missing then disk would be treated as without label. Label from boot sector would not be read.

The volume label format reference from Pali Rohár testing results
(https://www.spinics.net/lists/kernel/msg2645732.html) in the previous mail.

chenchacha (2):
Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl() Add a new function to get root directory with ATTR_VOLUME

 fs/fat/dir.c                  | 13 +++++++++++++
 fs/fat/fat.h                  |  2 ++
 fs/fat/file.c                 | 22 ++++++++++++++++++++++
 include/uapi/linux/msdos_fs.h |  1 +
 4 files changed, 38 insertions(+)
Signature · 1 line
2.19.0
#1 lore raw
Quoted 5 lines · On Friday 05 October 2018 01:20:59 chenchacha wrote:

This patch add fat ioctl command FAT_IOCTL_GET_VOLUME_LABEL.

We can get the volume label with this command:

	ioctl(fd, FAT_IOCTL_GET_VOLUME_LABEL, &volume_label);

Hi! What is purpose of having such functionality in kernel? Userspace can read it without any kernel support. (e.g. mlabel or fatlabel)

Quoted 18 lines

FAT volume label (volume name) is exactly same stored in boot sector and root directory. And this patch read label only from the root directory. If label in root directory is missing then disk would be treated as without label. Label from boot sector would not be read.

The volume label format reference from Pali Rohár testing results
(https://www.spinics.net/lists/kernel/msg2645732.html) in the previous mail.

chenchacha (2):
Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl() Add a new function to get root directory with ATTR_VOLUME

 fs/fat/dir.c                  | 13 +++++++++++++
 fs/fat/fat.h                  |  2 ++
 fs/fat/file.c                 | 22 ++++++++++++++++++++++
 include/uapi/linux/msdos_fs.h |  1 +
 4 files changed, 38 insertions(+)
Signature · 2 lines
Pali Rohár
[email protected]
author
#2 lore raw

PATCH 2/2 Add a new function to get root directory with ATTR_VOLUME

In reply to chenchacha
Signed-off-by chenchacha <[email protected]>
---
 fs/fat/dir.c | 13 +++++++++++++
 fs/fat/fat.h |  2 ++
 2 files changed, 15 insertions(+)
fs/fat/dir.c +13 -0
index 7f5f3699fc6c..4fdcc1200f2b 100644
@@ -881,6 +881,19 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
 	return -ENOENT;
 }
 
+int fat_get_root_entry(struct inode *dir, struct buffer_head **bh,
+		       struct msdos_dir_entry **de)
+{
+	loff_t offset = 0;
+
+	*de = NULL;
+	while (fat_get_entry(dir, &offset, bh, de) >= 0) {
+		if (!IS_FREE((*de)->name) && (*de)->attr & ATTR_VOLUME)
+			return 0;
+	}
+	return -ENOENT;
+}
+
 /*
  * The ".." entry can not provide the "struct fat_slot_info" information
  * for inode, nor a usable i_pos. So, this function provides some information
fs/fat/fat.h +2 -0
index be012de96f65..4195cb1e891a 100644
@@ -302,6 +302,8 @@ extern int fat_scan(struct inode *dir, const unsigned char *name,
 		    struct fat_slot_info *sinfo);
 extern int fat_scan_logstart(struct inode *dir, int i_logstart,
 			     struct fat_slot_info *sinfo);
+extern int fat_get_root_entry(struct inode *dir, struct buffer_head **bh,
+			      struct msdos_dir_entry **de);
 extern int fat_get_dotdot_entry(struct inode *dir, struct buffer_head **bh,
 				struct msdos_dir_entry **de);
 extern int fat_alloc_new_dir(struct inode *dir, struct timespec64 *ts);
Signature · 1 line
2.19.0
#3 lore raw

Re: [PATCH 2/2] Add a new function to get root directory with ATTR_VOLUME

Quoted 11 lines · On Friday 05 October 2018 01:21:01 chenchacha wrote:
Signed-off-by chenchacha <[email protected]>
---
 fs/fat/dir.c | 13 +++++++++++++
 fs/fat/fat.h |  2 ++
 2 files changed, 15 insertions(+)
fs/fat/dir.c +0 -0
index 7f5f3699fc6c..4fdcc1200f2b 100644

fs/fat/dir.c static int fat_get_short_entry(struct inode *dir, loff_t *pos, · line 885

Quoted 5 lines
fs/fat/dir.c +2 -0
@@ -881,6 +881,19 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
 	return -ENOENT;
 }
 
+int fat_get_root_entry(struct inode *dir, struct buffer_head **bh,
+		       struct msdos_dir_entry **de)

fat_get_root_volume_entry would be better name. It does not return root entry, but volume label entry.

fs/fat/dir.c static int fat_get_short_entry(struct inode *dir, loff_t *pos, · line 891

Quoted 6 lines
fs/fat/dir.c +6 -0
+{
+	loff_t offset = 0;
+
+	*de = NULL;
+	while (fat_get_entry(dir, &offset, bh, de) >= 0) {
+		if (!IS_FREE((*de)->name) && (*de)->attr & ATTR_VOLUME)

You should check that cluster number is zero. As e.g. first entry with ATTR_VOLUME can be also LFN entry and not real volume label. So you should properly check mask and filter out also LFN entries which have ATTR_VOLUME flag set too.

Quoted 24 lines
fs/fat/dir.c +5 -0
+			return 0;
+	}
+	return -ENOENT;
+}
+
 /*
  * The ".." entry can not provide the "struct fat_slot_info" information
  * for inode, nor a usable i_pos. So, this function provides some information
fs/fat/fat.h +2 -0
index be012de96f65..4195cb1e891a 100644
@@ -302,6 +302,8 @@ extern int fat_scan(struct inode *dir, const unsigned char *name,
 		    struct fat_slot_info *sinfo);
 extern int fat_scan_logstart(struct inode *dir, int i_logstart,
 			     struct fat_slot_info *sinfo);
+extern int fat_get_root_entry(struct inode *dir, struct buffer_head **bh,
+			      struct msdos_dir_entry **de);
 extern int fat_get_dotdot_entry(struct inode *dir, struct buffer_head **bh,
 				struct msdos_dir_entry **de);
 extern int fat_alloc_new_dir(struct inode *dir, struct timespec64 *ts);
Signature · 1 line
2.19.0
Signature · 2 lines
Pali Rohár
[email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

Re: [PATCH 1/2] Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl()

Quoted 16 lines · On Friday 05 October 2018 01:21:00 chenchacha wrote:
Signed-off-by chenchacha <[email protected]>
---
 fs/fat/file.c                 | 22 ++++++++++++++++++++++
 include/uapi/linux/msdos_fs.h |  1 +
 2 files changed, 23 insertions(+)
fs/fat/file.c +2 -0
index 4724cc9ad650..56db0b5a8df1 100644
@@ -121,10 +121,30 @@ 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 *label)
+{

fs/fat/file.c static int fat_ioctl_get_volume_id(struct inode *inode, u32 __user *user_attr) · line 135

Quoted 10 lines
fs/fat/file.c +10 -0
+	struct super_block *sb = inode->i_sb;
+	struct inode *root_inode = d_inode(sb->s_root);
+	struct buffer_head *bh = NULL;
+	struct msdos_dir_entry *de;
+	int err;
+
+	inode_lock_shared(root_inode);
+	err = fat_get_root_entry(root_inode, &bh, &de);
+	if (err == 0) {
+		if (copy_to_user(label, de->name, MSDOS_NAME))

You need to convert entry name from 8.3 format to label in correct encoding specified by codepage mount option. Plus needs to handle leading 0x03 byte.

Quoted 36 lines
fs/fat/file.c +10 -0
+			err = -EFAULT;
+	}
+	inode_unlock_shared(root_inode);
+
+	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 +153,8 @@ 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);
 	default:
 		return -ENOTTY;	/* Inappropriate ioctl for device */
 	}
include/uapi/linux/msdos_fs.h +1 -0
index fde753735aba..8ba8e2d7bafe 100644
@@ -109,6 +109,7 @@ 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[MSDOS_NAME])
 
 struct fat_boot_sector {
 	__u8	ignored[3];	/* Boot strap short or near jump */
Signature · 2 lines
Pali Rohár
[email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

Re: [PATCH 1/2] Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl()

Quoted 35 lines · On Saturday 06 October 2018 16:33:10 chen.chenchacha wrote:
Quoted 25 lines · Pali Rohár
Quoted 24 lines · On Friday 05 October 2018 01:21:00 chenchacha wrote:
Signed-off-by chenchacha <[email protected]>
---
 fs/fat/file.c                 | 22 ++++++++++++++++++++++
 include/uapi/linux/msdos_fs.h |  1 +
 2 files changed, 23 insertions(+)
fs/fat/file.c +8 -0
index 4724cc9ad650..56db0b5a8df1 100644
@@ -121,10 +121,30 @@ 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 *label)
+{
+	struct super_block *sb = inode->i_sb;
+	struct inode *root_inode = d_inode(sb->s_root);
+	struct buffer_head *bh = NULL;
+	struct msdos_dir_entry *de;
+	int err;
+
Quoted 9 lines

fs/fat/file.c static int fat_ioctl_get_volume_id(struct · line 137

Quoted 4 lines
fs/fat/file.c +4 -0
+	inode_lock_shared(root_inode);
+	err = fat_get_root_entry(root_inode, &bh, &de);
+	if (err == 0) {
+		if (copy_to_user(label, de->name, MSDOS_NAME))

You need to convert entry name from 8.3 format to label in correct encoding specified by codepage mount option. Plus needs to handle leading 0x03 byte.

Quoted 2 lines

I think it might be better to put the decoder/encoder on the application layter.

I do not think so. On all other places in msdos.ko and vfat.ko driver which communicate with userspace is that decoder/encoder active and user does not see raw bytes.

It is really bad to mix encodings and API of different calls.

If in mount option I specified that I want to use XYZ encoding, why driver does not going to respect it?

Quoted 6 lines

Similarly, the handle leading 0x03 byte is in the
write function. And make read operation pure, just to carry the volume label in root directory to the user.

What do you think, Pali?

Signature · 2 lines
Pali Rohár
[email protected]