Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/2] exfat: add dir-entry set checksum validation
To:     Namjae Jeon <namjae.jeon@samsung.com>
Cc:     kohada.tetsuhiro@dc.mitsubishielectric.co.jp,
        mori.takahiro@ab.mitsubishielectric.co.jp,
        motai.hirotaka@aj.mitsubishielectric.co.jp,
        'Sungjong Seo' <sj1557.seo@samsung.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
References: <CGME20200807073101epcas1p43ba30d0ff54cb09f90b7dc69c746d3e6@epcas1p4.samsung.com>
 <20200807073049.24959-1-kohada.t2@gmail.com>
 <003e01d66ede$2c2fe620$848fb260$@samsung.com>
From:   Tetsuhiro Kohada <kohada.t2@gmail.com>
Message-ID: <55c049e9-f32a-486b-b150-79bf1fbb886c@gmail.com>
Date:   Thu, 13 Aug 2020 00:17:05 +0900
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <003e01d66ede$2c2fe620$848fb260$@samsung.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Thnak you for your reply.

>> -void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
>> +static int exfat_calc_dir_chksum_with_entry_set(struct
>> +exfat_entry_set_cache *es, u16 *chksum)
>>   {
>> -	int chksum_type = CS_DIR_ENTRY, i;
>> -	unsigned short chksum = 0;
>>   	struct exfat_dentry *ep;
>> +	int i;
>>
>> -	for (i = 0; i < es->num_entries; i++) {
>> -		ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
>> -		chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
>> -					     chksum_type);
>> -		chksum_type = CS_DEFAULT;
>> +	ep = container_of(es->de_file, struct exfat_dentry, dentry.file);
>> +	*chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, 0, CS_DIR_ENTRY);
>> +	for (i = 0; i < es->de_file->num_ext; i++) {
>> +		ep = exfat_get_validated_dentry(es, 1 + i, TYPE_SECONDARY);
>> +		if (!ep)
>> +			return -EIO;
>> +		*chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, *chksum, CS_DEFAULT);
>>   	}
>> +	return 0;
> We can return checksum after removing u16 *chksum argument.

I want to do that too!
How should I return the error?

Right now, I found that the type of chksum is not 'u16'.
I'll fix in v2.


>> +}
>> +
>> +void exfat_update_dir_chksum_with_entry_set(struct
>> +exfat_entry_set_cache *es) {
>> +	u16 chksum;
>> +
>> +	exfat_calc_dir_chksum_with_entry_set(es, &chksum);
>>   	es->de_file->checksum = cpu_to_le16(chksum);
>>   	es->modified = true;
>>   }
>> @@ -775,6 +784,7 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
>>   	struct exfat_entry_set_cache *es;
>>   	struct exfat_dentry *ep;
>>   	struct buffer_head *bh;
>> +	u16 chksum;
>>
>>   	if (p_dir->dir == DIR_DELETED) {
>>   		exfat_err(sb, "access to deleted dentry"); @@ -839,10 +849,10 @@ struct
>> exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
>>   		goto free_es;
>>   	es->de_stream = &ep->dentry.stream;
>>
>> -	for (i = 2; i < es->num_entries; i++) {
>> -		if (!exfat_get_validated_dentry(es, i, TYPE_SECONDARY))
>> -			goto free_es;
>> -	}
>> +	if (max_entries == ES_ALL_ENTRIES &&
>> +	    ((exfat_calc_dir_chksum_with_entry_set(es, &chksum) ||
>> +	      chksum != le16_to_cpu(es->de_file->checksum))))
> Please add error print log if checksum mismatch error happen.

OK.
I'll add in v2.


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