2018-01-10 10:24:31

by chenchacha

[permalink] [raw]
Subject: [PATCH v7 0/3] fs: fat: add ioctl to modify fat filesystem partion volume label

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.

v7:
1. fixed volume label name check function.
2. automatically local rootdir.
3. remove some lock.
4. fixed some memory leaks.
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 | 53 ++++++++++++++++
fs/fat/fat.h | 6 ++
fs/fat/file.c | 140 ++++++++++++++++++++++++++++++++++++++++++
fs/fat/inode.c | 15 ++++-
include/uapi/linux/msdos_fs.h | 2 +
5 files changed, 213 insertions(+), 3 deletions(-)

--
2.11.0



2018-01-10 10:24:34

by chenchacha

[permalink] [raw]
Subject: [PATCH v7 1/3] fs: fat: Add fat filesystem partition volume label in local structure

Signed-off-by: ChenGuanqiao <[email protected]>
---
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 8fc1093da47d..96f631282c71 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -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;
@@ -300,6 +301,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_get_volume_label_entry(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 ffbbf0520d9e..b76d783138fd 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -46,12 +46,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;
@@ -1461,12 +1463,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) {
@@ -1724,11 +1730,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 a45d0754102e..90d2d6f3d908 100644
--- a/include/uapi/linux/msdos_fs.h
+++ b/include/uapi/linux/msdos_fs.h
@@ -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 _IOW('r', 0x15, __u8[11])

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


2018-01-10 10:24:42

by chenchacha

[permalink] [raw]
Subject: [PATCH v7 2/3] fs: fat: Add volume label entry method function

Signed-off-by: ChenGuanqiao <[email protected]>
---
fs/fat/dir.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 8e100c3bf72c..53f36c03760e 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -881,6 +881,59 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
return -ENOENT;
}

+int fat_get_volume_label_entry(struct inode *dir, struct buffer_head **bh,
+ struct msdos_dir_entry **de)
+{
+ loff_t pos = 0;
+
+ if (dir->i_ino != MSDOS_ROOT_INO)
+ return -EFAULT;
+
+ *bh = NULL;
+ *de = NULL;
+ while (fat_get_entry(dir, &pos, bh, de) >= 0) {
+ if (((*de)->attr & ATTR_VOLUME) && ((*de)->attr != ATTR_EXT) &&
+ !IS_FREE((*de)->name))
+ return 0;
+ }
+ return -ENOENT;
+}
+EXPORT_SYMBOL_GPL(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 -EFAULT;
+
+ 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;
+
+ brelse(sinfo.bh);
+
+ return 0;
+}
+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


2018-01-10 11:07:08

by chenchacha

[permalink] [raw]
Subject: [PATCH v7 3/3] fs: fat: add ioctl method in fat filesystem driver

Signed-off-by: ChenGuanqiao <[email protected]>
---
fs/fat/file.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)

diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4724cc9ad650..a46df684dba9 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -20,6 +20,25 @@
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 '_':
+ case 0x20:
+ continue;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int fat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
{
u32 attr;
@@ -121,10 +140,127 @@ 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;
+ struct super_block *sb = inode->i_sb;
+ char buffer[MSDOS_NAME] = {0};
+
+ inode = d_inode(sb->s_root);
+
+ err = fat_get_volume_label_entry(inode, &bh, &de);
+ if (err)
+ goto out;
+
+ inode_lock_shared(inode);
+ memcpy(buffer, de->name, MSDOS_NAME);
+ brelse(bh);
+ inode_unlock_shared(inode);
+
+ if (copy_to_user(vol_label, buffer, MSDOS_NAME))
+ err = -EFAULT;
+
+out:
+ return err;
+}
+
+static int fat_ioctl_set_volume_label(struct file *file,
+ u8 __user *vol_label)
+{
+ int err = 0;
+ u8 label[MSDOS_NAME];
+ struct timespec ts;
+ 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);
+
+ inode = d_inode(sb->s_root);
+
+ 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_get_volume_label_entry(inode, &vol_bh, &de);
+ ts = current_time(inode);
+ if (err) {
+ /* Create volume label entry */
+ err = fat_add_volume_label_entry(inode, label, &ts);
+ if (err)
+ goto out_vol_brelse;
+ } else {
+ /* Write to root directory */
+ memcpy(de->name, label, sizeof(de->name));
+ inode->i_ctime = inode->i_mtime = inode->i_atime = ts;
+
+ mark_buffer_dirty(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;
+ 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);
+
+ /* 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 +269,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

2018-01-14 12:50:54

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v7 2/3] fs: fat: Add volume label entry method function

ChenGuanqiao <[email protected]> writes:

> Signed-off-by: ChenGuanqiao <[email protected]>
> ---
> fs/fat/dir.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 8e100c3bf72c..53f36c03760e 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -881,6 +881,59 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
> return -ENOENT;
> }
>
> +int fat_get_volume_label_entry(struct inode *dir, struct buffer_head **bh,
> + struct msdos_dir_entry **de)
> +{
> + loff_t pos = 0;
> +
> + if (dir->i_ino != MSDOS_ROOT_INO)
> + return -EFAULT;

Now, dir is always root, so this check doesn't matter.

> + *bh = NULL;
> + *de = NULL;
> + while (fat_get_entry(dir, &pos, bh, de) >= 0) {
> + if (((*de)->attr & ATTR_VOLUME) && ((*de)->attr != ATTR_EXT) &&
> + !IS_FREE((*de)->name))
> + return 0;
> + }
> + return -ENOENT;
> +}
> +EXPORT_SYMBOL_GPL(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 -EFAULT;

Here is too. dir is always root.

> + 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;
> +
> + brelse(sinfo.bh);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(fat_add_volume_label_entry);

--
OGAWA Hirofumi <[email protected]>

2018-01-14 13:17:48

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v7 3/3] fs: fat: add ioctl method in fat filesystem driver

ChenGuanqiao <[email protected]> writes:

> +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 '_':
> + case 0x20:
> + continue;

I didn't check though, what windows do if label = "a b c"? (I.e. invalid
name as dirent name)

> +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;
> + struct super_block *sb = inode->i_sb;
> + char buffer[MSDOS_NAME] = {0};

Why initialize buffer?

> + inode = d_inode(sb->s_root);
> +
> + err = fat_get_volume_label_entry(inode, &bh, &de);
> + if (err)
> + goto out;

The dir traverse has to be had lock.

> + inode_lock_shared(inode);
> + memcpy(buffer, de->name, MSDOS_NAME);
> + brelse(bh);
> + inode_unlock_shared(inode);
> +
> + if (copy_to_user(vol_label, buffer, MSDOS_NAME))
> + err = -EFAULT;

No issue to copy to user memory under locking.

> +static int fat_ioctl_set_volume_label(struct file *file,
> + u8 __user *vol_label)
> +{

[...]

> + 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);

BTW, lock order is tested with LOCKDEP?

> + /* 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;

Probably, we don't need to sync vol_bh. And in the case of adding new
entry, now already doesn't sync.
--
OGAWA Hirofumi <[email protected]>

2018-01-14 13:18:52

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] fs: fat: Add fat filesystem partition volume label in local structure

ChenGuanqiao <[email protected]> writes:

> Signed-off-by: ChenGuanqiao <[email protected]>
> ---
> 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 8fc1093da47d..96f631282c71 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -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*/

In latter patches doesn't use this cache. Why do we cache, or why read
ioctl doesn't use this cache?
--
OGAWA Hirofumi <[email protected]>