diff mbox series

[1/2] exfat: add dir-entry set checksum validation

Message ID 20200807073049.24959-1-kohada.t2@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] exfat: add dir-entry set checksum validation | expand

Commit Message

Tetsuhiro Kohada Aug. 7, 2020, 7:30 a.m. UTC
Add checksum validation for dir-entry set when getting it.
exfat_calc_dir_chksum_with_entry_set() also validates entry-type.

** This patch depends on:
  '[PATCH v3] exfat: integrates dir-entry getting and validation'

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/dir.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

Namjae Jeon Aug. 10, 2020, 6:19 a.m. UTC | #1
> Add checksum validation for dir-entry set when getting it.
> exfat_calc_dir_chksum_with_entry_set() also validates entry-type.
> 
> ** This patch depends on:
>   '[PATCH v3] exfat: integrates dir-entry getting and validation'
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> ---
>  fs/exfat/dir.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index c9715c7a55a1..2e79ac464f5f 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -563,18 +563,27 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
>  	return 0;
>  }
> 
> -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.
> +}
> +
> +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.
> +		goto free_es;
> 
>  	return es;
> 
> --
> 2.25.1
Tetsuhiro Kohada Aug. 12, 2020, 3:17 p.m. UTC | #2
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>
diff mbox series

Patch

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index c9715c7a55a1..2e79ac464f5f 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -563,18 +563,27 @@  int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
 	return 0;
 }
 
-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;
+}
+
+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))))
+		goto free_es;
 
 	return es;