linux-kernel - Re: [PATCH 2/2] exfat: unify name extraction

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 for Android: free password hash cracker in your pocket

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

Message-ID: <bbd9355c-cd48-b961-0a91-771a702c03df@gmail.com>
Date:   Wed, 12 Aug 2020 15:02:17 +0900
From:   Tetsuhiro Kohada <kohada.t2@...il.com>
To:     Sungjong Seo <sj1557.seo@...sung.com>
Cc:     kohada.tetsuhiro@...mitsubishielectric.co.jp,
        mori.takahiro@...mitsubishielectric.co.jp,
        motai.hirotaka@...mitsubishielectric.co.jp,
        'Namjae Jeon' <namjae.jeon@...sung.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] exfat: unify name extraction

Thanks for your reply.

On 2020/08/09 2:19, Sungjong Seo wrote:
> [snip]
>> @@ -963,80 +942,38 @@ int exfat_find_dir_entry(struct super_block *sb,
>> struct exfat_inode_info *ei,
>>   			num_empty = 0;
>>   			candi_empty.eidx = EXFAT_HINT_NONE;
>>
> [snip]
>>
>> -			if (entry_type &
>> -					(TYPE_CRITICAL_SEC |
> TYPE_BENIGN_SEC)) {
>> -				if (step == DIRENT_STEP_SECD) {
>> -					if (++order == num_ext)
>> -						goto found;
>> -					continue;
>> -				}
>> +			exfat_get_uniname_from_name_entries(es, &uni_name);
> 
> It is needed to check a return value.

I'll fix it in v2.

>> +			exfat_free_dentry_set(es, false);
>> +
>> +			if (!exfat_uniname_ncmp(sb,
>> +						p_uniname->name,
>> +						uni_name.name,
>> +						name_len)) {
>> +				/* set the last used position as hint */
>> +				hint_stat->clu = clu.dir;
>> +				hint_stat->eidx = dentry;
> 
> eidx and clu of hint_stat should have one for the next entry we'll start
> looking for.
> Did you intentionally change the concept?

Yes, this is intentional.
Essentially, the "Hint" concept is to reduce the next seek cost with minimal cost.
There is a difference in the position of the hint, but the concept is the same.
As you can see, the patched code strategy doesn't move from current position.
Basically, the original code strategy is advancing only one dentry.(It's the "minimum cost")
However, when it reaches the cluster boundary, it gets the next cluster and error handling.
Getting the next cluster The error handling already exists at the end of the while loop,
so the code is duplicated.
These costs should be paid next time and are no longer the "minimum cost".

Should I add this to the commit-message?

BR
---
Tetsuhiro Kohada <kohada.t2@...il.com>