[PATCH v1] exfat: zero the reserved fields of file and stream extension dentries

[PATCH v1] exfat: zero the reserved fields of file and stream extension dentries

[Yuezhang.Mo]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

From exFAT specification, the reserved fields should initialize
to zero and should not use for any purpose.

If create a new dentry set in the UNUSED dentries, all fields
had been zeroed when allocating cluster to parent directory.

But if create a new dentry set in the DELETED dentries, the
reserved fields in file and stream extension dentries may be
non-zero. Because only the valid bit of the type field of the
dentry is cleared in exfat_remove_entries(), if the type of
dentry is different from the original(For example, a dentry that
was originally a file name dentry, then set to deleted dentry,
and then set as a file dentry), the reserved fields is non-zero.

So this commit zeroes the reserved fields when createing file
dentry and stream extension dentry.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
---
 fs/exfat/dir.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 077944d3c2c0..cbdd9b59053d 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -428,6 +428,10 @@ static void exfat_init_stream_entry(struct exfat_dentry *ep,
 	ep->dentry.stream.start_clu = cpu_to_le32(start_clu);
 	ep->dentry.stream.valid_size = cpu_to_le64(size);
 	ep->dentry.stream.size = cpu_to_le64(size);
+
+	ep->dentry.stream.reserved1 = 0;
+	ep->dentry.stream.reserved2 = 0;
+	ep->dentry.stream.reserved3 = 0;
 }
 
 static void exfat_init_name_entry(struct exfat_dentry *ep,
@@ -474,6 +478,9 @@ void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
 			&ep->dentry.file.access_date,
 			NULL);
 
+	ep->dentry.file.reserved1 = 0;
+	memset(ep->dentry.file.reserved2, 0, sizeof(ep->dentry.file.reserved2));
+
 	ep = exfat_get_dentry_cached(es, ES_IDX_STREAM);
 	exfat_init_stream_entry(ep, start_clu, size);
 }
-- 
2.34.1

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v1-0001-exfat-zero-the-reserved-fields-of-file-and-stream.patch --]
[-- Type: text/x-patch; name="v1-0001-exfat-zero-the-reserved-fields-of-file-and-stream.patch", Size: 2105 bytes --]

From a44117862361870f994029f8a8424905fe3cc0f0 Mon Sep 17 00:00:00 2001
From: Yuezhang Mo <Yuezhang.Mo@sony.com>
Date: Fri, 12 Jan 2024 14:48:46 +0800
Subject: [PATCH v1] exfat: zero the reserved fields of file and stream
 extension dentries

From exFAT specification, the reserved fields should initialize
to zero and should not use for any purpose.

If create a new dentry set in the UNUSED dentries, all fields
had been zeroed when allocating cluster to parent directory.

But if create a new dentry set in the DELETED dentries, the
reserved fields in file and stream extension dentries may be
non-zero. Because only the valid bit of the type field of the
dentry is cleared in exfat_remove_entries(), if the type of
dentry is different from the original(For example, a dentry that
was originally a file name dentry, then set to deleted dentry,
and then set as a file dentry), the reserved fields is non-zero.

So this commit zeroes the reserved fields when createing file
dentry and stream extension dentry.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
---
 fs/exfat/dir.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 077944d3c2c0..cbdd9b59053d 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -428,6 +428,10 @@ static void exfat_init_stream_entry(struct exfat_dentry *ep,
 	ep->dentry.stream.start_clu = cpu_to_le32(start_clu);
 	ep->dentry.stream.valid_size = cpu_to_le64(size);
 	ep->dentry.stream.size = cpu_to_le64(size);
+
+	ep->dentry.stream.reserved1 = 0;
+	ep->dentry.stream.reserved2 = 0;
+	ep->dentry.stream.reserved3 = 0;
 }
 
 static void exfat_init_name_entry(struct exfat_dentry *ep,
@@ -474,6 +478,9 @@ void exfat_init_dir_entry(struct exfat_entry_set_cache *es,
 			&ep->dentry.file.access_date,
 			NULL);
 
+	ep->dentry.file.reserved1 = 0;
+	memset(ep->dentry.file.reserved2, 0, sizeof(ep->dentry.file.reserved2));
+
 	ep = exfat_get_dentry_cached(es, ES_IDX_STREAM);
 	exfat_init_stream_entry(ep, start_clu, size);
 }
-- 
2.34.1

RE: [PATCH v1] exfat: zero the reserved fields of file and stream extension dentries

[Sungjong Seo]

> From exFAT specification, the reserved fields should initialize
> to zero and should not use for any purpose.
> 
> If create a new dentry set in the UNUSED dentries, all fields
> had been zeroed when allocating cluster to parent directory.
> 
> But if create a new dentry set in the DELETED dentries, the
> reserved fields in file and stream extension dentries may be
> non-zero. Because only the valid bit of the type field of the
> dentry is cleared in exfat_remove_entries(), if the type of
> dentry is different from the original(For example, a dentry that
> was originally a file name dentry, then set to deleted dentry,
> and then set as a file dentry), the reserved fields is non-zero.
> 
> So this commit zeroes the reserved fields when createing file
> dentry and stream extension dentry.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
> ---
>  fs/exfat/dir.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
> index 077944d3c2c0..cbdd9b59053d 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -428,6 +428,10 @@ static void exfat_init_stream_entry(struct
> exfat_dentry *ep,
>  	ep->dentry.stream.start_clu = cpu_to_le32(start_clu);
>  	ep->dentry.stream.valid_size = cpu_to_le64(size);
>  	ep->dentry.stream.size = cpu_to_le64(size);
> +
> +	ep->dentry.stream.reserved1 = 0;
> +	ep->dentry.stream.reserved2 = 0;
> +	ep->dentry.stream.reserved3 = 0;

The comment explains the problem well! And the patch you just sent
seems to solve the mentioned problem.

BTW, what about initializing the entire ep (fixed size of 32 bytes)
to 0 before setting the value of ep in each init function? This is the
simplest way to ensure that all other values are zero except for the
intentionally set value.

>  }
> 
>  static void exfat_init_name_entry(struct exfat_dentry *ep,
> @@ -474,6 +478,9 @@ void exfat_init_dir_entry(struct exfat_entry_set_cache
> *es,
>  			&ep->dentry.file.access_date,
>  			NULL);
> 
> +	ep->dentry.file.reserved1 = 0;
> +	memset(ep->dentry.file.reserved2, 0, sizeof(ep-
> >dentry.file.reserved2));
> +
>  	ep = exfat_get_dentry_cached(es, ES_IDX_STREAM);
>  	exfat_init_stream_entry(ep, start_clu, size);
>  }
> --
> 2.34.1

Re: [PATCH v1] exfat: zero the reserved fields of file and stream extension dentries

[Yuezhang.Mo]

> BTW, what about initializing the entire ep (fixed size of 32 bytes)
> to 0 before setting the value of ep in each init function? This is the
> simplest way to ensure that all other values are zero except for the
> intentionally set value.

Yes, initializing the entire directory entry to 0 is simplest way.
But 48 more bytes are set to 0 (the total size of the reserved fields is
16 bytes).

I think both ways are acceptable. If you think initializing the entire ep to
0 is better, I'll update this patch.

RE: [PATCH v1] exfat: zero the reserved fields of file and stream extension dentries

[Sungjong Seo]

> > BTW, what about initializing the entire ep (fixed size of 32 bytes)
> > to 0 before setting the value of ep in each init function? This is the
> > simplest way to ensure that all other values are zero except for the
> > intentionally set value.
> 
> Yes, initializing the entire directory entry to 0 is simplest way.
> But 48 more bytes are set to 0 (the total size of the reserved fields is
> 16 bytes).
> 
> I think both ways are acceptable. If you think initializing the entire ep
> to
> 0 is better, I'll update this patch.
It may be more efficient to initialize the entire ep of size 32 bytes at
once.
I will wait for patch v2. :)