2020-08-07 07:31:57

by Tetsuhiro Kohada

[permalink] [raw]
Subject: [PATCH 1/2] exfat: add dir-entry set checksum validation

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 <[email protected]>
---
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;
+}
+
+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;

--
2.25.1


2020-08-07 07:34:15

by Tetsuhiro Kohada

[permalink] [raw]
Subject: [PATCH 2/2] exfat: write only modified part of dir-entry set

Currently exfat_free_dentry_set() writes all of dir-entry set.
Change it to write only the modified part of dir-entry set.
And, Integrate exfat_free_dentry_set() and
exfat_update_dir_chksum_with_entry_set() as exfat_put_dentry_set().

** This patch depends on:
'[PATCH v3] exfat: integrates dir-entry getting and validation'
'[PATCH] exfat: add NameLength check when extracting name'
'[PATCH] exfat: unify name extraction'

Signed-off-by: Tetsuhiro Kohada <[email protected]>
---
fs/exfat/dir.c | 33 ++++++++++++++++-----------------
fs/exfat/exfat_fs.h | 4 +---
fs/exfat/file.c | 3 +--
fs/exfat/inode.c | 6 ++----
fs/exfat/namei.c | 4 ++--
5 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 2e79ac464f5f..5071a05f1150 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -133,7 +133,7 @@ static int exfat_readdir(struct inode *inode, struct exfat_dir_entry *dir_entry)
dir_entry->namebuf.lfn,
dir_entry->namebuf.lfnbuf_len);

- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);

ei->hint_bmap.off = dentry >> dentries_per_clu_bits;
ei->hint_bmap.clu = clu.dir;
@@ -579,21 +579,21 @@ static int exfat_calc_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es
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;
-}
-
-int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync)
+int exfat_put_dentry_set(struct exfat_entry_set_cache *es, int modified, int sync)
{
int i, err = 0;

- if (es->modified)
- err = exfat_update_bhs(es->bh, es->num_bh, sync);
+ if (modified) {
+ int off = es->start_off + (modified - 1) * DENTRY_SIZE;
+ int modified_bh = min(EXFAT_B_TO_BLK(off, es->sb) + 1, es->num_bh);
+ u16 chksum;
+
+ err = exfat_calc_dir_chksum_with_entry_set(es, &chksum);
+ if (!err) {
+ es->de_file->checksum = cpu_to_le16(chksum);
+ err = exfat_update_bhs(es->bh, modified_bh, sync);
+ }
+ }

for (i = 0; i < es->num_bh; i++)
if (err)
@@ -800,7 +800,6 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
if (!es)
return NULL;
es->sb = sb;
- es->modified = false;
es->num_entries = 1;

/* byte offset in cluster */
@@ -857,7 +856,7 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
return es;

free_es:
- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);
return NULL;
}

@@ -962,7 +961,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
num_ext = es->de_file->num_ext;
name_hash = le16_to_cpu(es->de_stream->name_hash);
name_len = es->de_stream->name_len;
- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);

if (p_uniname->name_hash != name_hash ||
p_uniname->name_len != name_len)
@@ -973,7 +972,7 @@ int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
continue;

exfat_get_uniname_from_name_entries(es, &uni_name);
- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);

if (!exfat_uniname_ncmp(sb,
p_uniname->name,
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 62a4768a4f6e..6fac1cec3e7f 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -170,7 +170,6 @@ struct exfat_hint {

struct exfat_entry_set_cache {
struct super_block *sb;
- bool modified;
unsigned int start_off;
int num_bh;
struct buffer_head *bh[DIR_CACHE_SIZE];
@@ -452,7 +451,6 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
int entry, int order, int num_entries);
int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
int entry);
-void exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es);
int exfat_calc_num_entries(struct exfat_uni_name *p_uniname);
int exfat_find_dir_entry(struct super_block *sb, struct exfat_inode_info *ei,
struct exfat_chain *p_dir, struct exfat_uni_name *p_uniname,
@@ -467,7 +465,7 @@ struct exfat_dentry *exfat_get_validated_dentry(struct exfat_entry_set_cache *es
int num, unsigned int type);
struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
struct exfat_chain *p_dir, int entry, int max_entries);
-int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync);
+int exfat_put_dentry_set(struct exfat_entry_set_cache *es, int modified, int sync);
int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir);

/* inode.c */
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 04f6cc79ed43..e1f330169929 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -185,8 +185,7 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
es->de_stream->start_clu = EXFAT_FREE_CLUSTER;
}

- exfat_update_dir_chksum_with_entry_set(es);
- err = exfat_free_dentry_set(es, inode_needs_sync(inode));
+ err = exfat_put_dentry_set(es, 2, inode_needs_sync(inode));
if (err)
return err;
}
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 358457c82270..894321b561e6 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -73,8 +73,7 @@ static int __exfat_write_inode(struct inode *inode, int sync)
es->de_stream->valid_size = cpu_to_le64(on_disk_size);
es->de_stream->size = es->de_stream->valid_size;

- exfat_update_dir_chksum_with_entry_set(es);
- return exfat_free_dentry_set(es, sync);
+ return exfat_put_dentry_set(es, 2, sync);
}

int exfat_write_inode(struct inode *inode, struct writeback_control *wbc)
@@ -230,8 +229,7 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset,
es->de_stream->valid_size = cpu_to_le64(i_size_read(inode));
es->de_stream->size = es->de_stream->valid_size;

- exfat_update_dir_chksum_with_entry_set(es);
- err = exfat_free_dentry_set(es, inode_needs_sync(inode));
+ err = exfat_put_dentry_set(es, 2, inode_needs_sync(inode));
if (err)
return err;
} /* end of if != DIR_DELETED */
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index c59d523547ca..0f559785f7cb 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -677,7 +677,7 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
exfat_fs_error(sb,
"non-zero size file starts with zero cluster (size : %llu, p_dir : %u, entry : 0x%08x)",
i_size_read(dir), ei->dir.dir, ei->entry);
- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);
return -EIO;
}

@@ -696,7 +696,7 @@ static int exfat_find(struct inode *dir, struct qstr *qname,
es->de_file->access_time,
es->de_file->access_date,
0);
- exfat_free_dentry_set(es, false);
+ exfat_put_dentry_set(es, 0, false);

if (info->type == TYPE_DIR) {
exfat_chain_set(&cdir, info->start_clu,
--
2.25.1

2020-08-10 06:22:06

by Namjae Jeon

[permalink] [raw]
Subject: RE: [PATCH 1/2] exfat: add dir-entry set checksum validation

> 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 <[email protected]>
> ---
> 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


2020-08-12 15:18:18

by Tetsuhiro Kohada

[permalink] [raw]
Subject: Re: [PATCH 1/2] exfat: add dir-entry set checksum validation

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 <[email protected]>