Messages in this thread | | | Subject | Re: [PATCH 1/2] exfat: add NameLength check when extracting name | From | Tetsuhiro Kohada <> | Date | Wed, 12 Aug 2020 13:53:12 +0900 |
| |
Thanks for your reply.
On 2020/08/09 1:54, Sungjong Seo wrote: >> The current implementation doesn't care NameLength when extracting the >> name from Name dir-entries, so the name may be incorrect. >> (Without null-termination, Insufficient Name dir-entry, etc) Add a >> NameLength check when extracting the name from Name dir-entries to extract >> correct name. >> And, change to get the information of file/stream-ext dir-entries via the >> member variable of exfat_entry_set_cache. >> >> ** 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 | 81 ++++++++++++++++++++++++-------------------------- >> 1 file changed, 39 insertions(+), 42 deletions(-) >> >> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index >> 91cdbede0fd1..545bb73b95e9 100644 >> --- a/fs/exfat/dir.c >> +++ b/fs/exfat/dir.c >> @@ -28,16 +28,15 @@ static int exfat_extract_uni_name(struct exfat_dentry >> *ep, >> >> } >> >> -static void exfat_get_uniname_from_ext_entry(struct super_block *sb, >> - struct exfat_chain *p_dir, int entry, unsigned short >> *uniname) >> +static int exfat_get_uniname_from_name_entries(struct >> exfat_entry_set_cache *es, >> + struct exfat_uni_name *uniname) >> { >> - int i; >> - struct exfat_entry_set_cache *es; >> + int n, l, i; >> struct exfat_dentry *ep; >> >> - es = exfat_get_dentry_set(sb, p_dir, entry, ES_ALL_ENTRIES); >> - if (!es) >> - return; >> + uniname->name_len = es->de_stream->name_len; >> + if (uniname->name_len == 0) >> + return -EIO; > > -EINVAL looks better.
OK. I'll change in v2.
>> /* >> * First entry : file entry >> @@ -45,14 +44,15 @@ static void exfat_get_uniname_from_ext_entry(struct >> super_block *sb, >> * Third entry : first file-name entry >> * So, the index of first file-name dentry should start from 2. >> */ >> - >> - i = 2; >> - while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) { >> - exfat_extract_uni_name(ep, uniname); >> - uniname += EXFAT_FILE_NAME_LEN; >> + for (l = 0, n = 2; l < uniname->name_len; n++) { >> + ep = exfat_get_validated_dentry(es, n, TYPE_NAME); >> + if (!ep) >> + return -EIO; >> + for (i = 0; l < uniname->name_len && i < > EXFAT_FILE_NAME_LEN; >> i++, l++) >> + uniname->name[l] = le16_to_cpu(ep- >>> dentry.name.unicode_0_14[i]); > > Looks good. > >> } >> - >> - exfat_free_dentry_set(es, false); >> + uniname->name[l] = 0; >> + return 0; >> } >> >> /* read a directory entry from the opened directory */ @@ -63,6 +63,7 @@ >> static int exfat_readdir(struct inode *inode, struct exfat_dir_entry >> *dir_entry) > [snip] >> - *uni_name.name = 0x0; >> - exfat_get_uniname_from_ext_entry(sb, &dir, dentry, >> - uni_name.name); >> + dir_entry->size = le64_to_cpu(es->de_stream- >>> valid_size); >> + >> + exfat_get_uniname_from_name_entries(es, &uni_name); > > Modified function has a return value. > It would be better to check the return value.
Oops! I'll fix it in v2.
>> exfat_utf16_to_nls(sb, &uni_name, >> dir_entry->namebuf.lfn, >> dir_entry->namebuf.lfnbuf_len); >> - brelse(bh); >> >> - ep = exfat_get_dentry(sb, &clu, i + 1, &bh, NULL); >> - if (!ep) >> - return -EIO; >> - dir_entry->size = >> - le64_to_cpu(ep->dentry.stream.valid_size); >> - brelse(bh); >> + exfat_free_dentry_set(es, false); >> >> ei->hint_bmap.off = dentry >> dentries_per_clu_bits; >> ei->hint_bmap.clu = clu.dir; >> -- >> 2.25.1 > >
| |