ChenGuanqiao <[email protected]> writes:

> +/* the characters in this field shall be d-characters, and unused byte shall 
> be set to 0x20. */
> +static int fat_format_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;
> +             default:
> +                     break;
> +             }
> +             break;
> +     }
> +
> +     for (; i<len; ++i)
> +             label[i] = 0x20;
> +
> +     return len;
> +}

It should not accept and overwrite invalid char, return -EINVAL.

> +static int fat_ioctl_get_volume_label(struct inode *inode,
> +                                   u8 __user *vol_label)
> +{
> +     int err = 0;
> +     struct fat_slot_info sinfo;
> +
> +     err = fat_scan_volume_label(inode, &sinfo);
> +     if (err)
> +             goto out;

It needs to take inode read lock.

> +     if (copy_to_user(vol_label, sinfo.de->name, MSDOS_NAME))
> +             err = -EFAULT;
> +
> +     brelse(sinfo.bh);
> +out:
> +     return err;
> +}
> +
> +static int fat_ioctl_set_volume_label(struct file *file,
> +                                   u8 __user *vol_label)
> +{

[...]

> +     b = (struct fat_boot_sector *)bh->b_data;
> +
> +     lock_buffer(bh);

Probably, sb->s_umount to exclusive with remount update.

> +     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);
> +     unlock_buffer(bh);
> +     err = sync_dirty_buffer(bh);
> +     brelse(bh);
> +     if (err)
> +             goto out_drop_file;

It should not update before preparing is done and checking error.

> +     /* updates root directory's vol_label */
> +     err = fat_scan_volume_label(inode, &sinfo);
> +     if (err)
> +             goto out_drop_file;

It should add entry if no volume label.

> +     lock_buffer(bh);

inode write lock.

> +     memcpy(sinfo.de->name, label, sizeof(sinfo.de->name));
> +     mark_buffer_dirty(bh);
> +     unlock_buffer(bh);

Thanks.
-- 
OGAWA Hirofumi <[email protected]>

Reply via email to