linux-kernel - Re: [PATCH 3/3] fs: fat: add ioctl method in fat filesystem driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bmhbmvzc.fsf@mail.parknet.co.jp>
Date:   Tue, 30 Jan 2018 20:23:03 +0900
From:   OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:     chenchacha <chen.chenchacha@...mail.com>
Cc:     linux-kernel@...r.kernel.org,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        pali.rohar@...il.com
Subject: Re: [PATCH 3/3] fs: fat: add ioctl method in fat filesystem driver

chenchacha <chen.chenchacha@...mail.com> writes:

> On 01/29/2018 09:02 PM, OGAWA Hirofumi wrote:
>> ChenGuanqiao <chen.chenchacha@...mail.com> writes:
>>
>>> +static int fat_check_d_characters(char *label, unsigned long len)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < len; ++i) {
>>> +		switch (label[i]) {
>>> +		case 'a' ... 'z':
>>> +			label[i] = __toupper(label[i]);
>>> +		case 'A' ... 'Z':
>>> +		case '0' ... '9':
>>> +		case '_':
>>> +		case 0x20:
>>> +			continue;
>>> +		default:
>>> +			return -EINVAL;
>>> +		}
>> Same question with previous though, what windows do if label = "a b c"?
>> (this is including space other than end of name or extension.)
> In win7, the volume label will be capitalized, and leaving spaces.
> Or, you mean I need to fill the rest of the space with "0x20"?

I see. However, what win7 stored, BTW? It was "A B C      ", or anything
other?

>>> +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);
>>> +
>> [...]
> I need remove "struct msdos_sb_info *sbi"?

If you didn't use sbi anymore, you should remove.

>>> +	err = mnt_want_write_file(file);
>>> +	if (err)
>>> +		goto out;
>>> +
>>> +	down_write(&sb->s_umount);
>> Looks like inode_lock() for rootdir is gone. It is necessary to
>> traverse+modify.
> Is it wrong for me to use the inode_lock() in patch v7? I need to lock 
> inode here, and turn off immediately after

inode_lock() is necessary to protect race with other dir operations.

I asked at v7, locking order to prevent the AB locking order bug.

I.e.
	mnt_want_write_file => down_write => inode_lock()
vs
	down_write => mnt_want_write_file => inode_lock()

Which is right one?

> mark_buffer_dirty(vol_bh)?

What is this asking?
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>