| Message ID | PUZPR04MB63168EFB1C670A913C42E80981112@PUZPR04MB6316.apcprd04.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] exfat: zero the reserved fields of file and stream extension dentries | expand |
> 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
> 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.
> > 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. :)
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