linux-kernel - Re: [PATCH] f2fs: add sanity check for quota sysfile ino

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-prev] [day] [month] [year] [list]

Message-ID: <4013a9fa-c667-f919-261d-51b353812b53@kernel.org>
Date:   Thu, 1 Feb 2018 21:41:04 +0800
From:   Chao Yu <chao@...nel.org>
To:     Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <yuchao0@...wei.com>
Cc:     linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: add sanity check for quota sysfile ino

On 2018/2/1 5:57, Jaegeuk Kim wrote:
> On 01/31, Chao Yu wrote:
>> On 2018/1/31 11:49, Jaegeuk Kim wrote:
>>> On 01/31, Chao Yu wrote:
>>>> On 2018/1/31 10:36, Jaegeuk Kim wrote:
>>>>> On 01/31, Chao Yu wrote:
>>>>>> On 2018/1/31 9:35, Jaegeuk Kim wrote:
>>>>>>> On 01/29, Chao Yu wrote:
>>>>>>>> Add missing sanity check for quota sysfile ino.
>>>>>>>
>>>>>>> We don't need to limit the specific inode numbers for quota files, since
>>>>>>> we may be able to set any inode numbers later. How about checkint the
>>>>>>> numbers are allocated as quota?
>>>>>
>>>>> If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
>>>>> It'd be also fine, if we have no inode numbers there. What I mean was we may need
>>>>> to consider reallocating or deallocating inode numbers later.
>>>>
>>>> Oh, so that it will be better to do more check & repair in fsck tools, right?
>>>
>>> What is the rule that you're considering? We're already checking quota inodes
>>> in fsck.f2fs.
>>
>> The case that qf_ino[] is corrupted, I didn't see it can be relinked to
>> corresponding quota inode by fsck, am I missing something?
> 
> We can check its basic metadata such as i_mode? Or, in order to do that

Agreed, how about:

if (i_mode == 0x8180 && i_flags == FS_IMMUTABLE_FL &&
	i_name[0] == 0x00 && i_namelen == 0)
	relink super_block::qf_ino to quote inode;

> entirely, we need to add file truncation and creationg in fsck.f2fs, which

Yes, I think we can rebuild sys quote file if original one is missing or corrupted.

Thanks,

> was too complicated to add before.
> Any idea?
> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Do you mean:
>>>>>>
>>>>>> 	if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>> 		if (!le32_to_cpu(raw_super->qf_ino[0]) ||
>>>>>> 			!le32_to_cpu(raw_super->qf_ino[1])) {
>>>>>> 			f2fs_msg(sb, KERN_INFO,
>>>>>> 				"Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>>>> 				le32_to_cpu(raw_super->qf_ino[0]),
>>>>>> 				le32_to_cpu(raw_super->qf_ino[1]));
>>>>>> 			return 1;
>>>>>> 		}
>>>>>> 		if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>>>> 			!le32_to_cpu(raw_super->qf_ino[2])) {
>>>>>> 			f2fs_msg(sb, KERN_INFO,
>>>>>> 				"Invalid Quota Ino: prj_ino(%u)",
>>>>>> 				le32_to_cpu(raw_super->qf_ino[2]));
>>>>>> 			return 1;
>>>>>> 		}
>>>>>> 	}
>>>>>>
>>>>>> I think it's equal to:
>>>>>>
>>>>>> 	if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>> 		unsigned int qf_cnt = 0;
>>>>>> 		if (qf_ino[0])
>>>>>> 			qf_cnt++;
>>>>>> 		if (qf_ino[1])
>>>>>> 			qf_cnt++;
>>>>>> 		if (qf_cnt != 2) {
>>>>>> 			f2fs_msg();
>>>>>> 			return 1;
>>>>>> 		}
>>>>>> 		if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>>>>>> 			if (qf_ino[2])
>>>>>> 				qf_cnt++;
>>>>>> 			if (qf_cnt != 3) {
>>>>>> 				f2fs_msg();
>>>>>> 				return 1;
>>>>>> 			}
>>>>>> 		}
>>>>>> 	}
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Chao Yu <yuchao0@...wei.com>
>>>>>>>> ---
>>>>>>>>  fs/f2fs/super.c | 19 +++++++++++++++++++
>>>>>>>>  1 file changed, 19 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>>>> index 368f63d7bad2..6011071688ca 100644
>>>>>>>> --- a/fs/f2fs/super.c
>>>>>>>> +++ b/fs/f2fs/super.c
>>>>>>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
>>>>>>>>  		return 1;
>>>>>>>>  	}
>>>>>>>>  
>>>>>>>> +	/* check quota sysfile ino info */
>>>>>>>> +	if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>>>> +		if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
>>>>>>>> +			le32_to_cpu(raw_super->qf_ino[1]) != 5) {
>>>>>>>> +			f2fs_msg(sb, KERN_INFO,
>>>>>>>> +				"Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>>>>>> +				le32_to_cpu(raw_super->qf_ino[0]),
>>>>>>>> +				le32_to_cpu(raw_super->qf_ino[1]));
>>>>>>>> +			return 1;
>>>>>>>> +		}
>>>>>>>> +		if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>>>>>> +			le32_to_cpu(raw_super->qf_ino[2]) != 6) {
>>>>>>>> +			f2fs_msg(sb, KERN_INFO,
>>>>>>>> +				"Invalid Quota Ino: prj_ino(%u)",
>>>>>>>> +				le32_to_cpu(raw_super->qf_ino[2]));
>>>>>>>> +			return 1;
>>>>>>>> +		}
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>>  	if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
>>>>>>>>  		f2fs_msg(sb, KERN_INFO,
>>>>>>>>  			"Invalid segment count (%u)",
>>>>>>>> -- 
>>>>>>>> 2.15.0.55.gc2ece9dc4de6
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>
>>>>> .
>>>>>
>>>
>>> .
>>>