public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Sungjong Seo" <sj1557.seo@samsung.com>
To: "'Tetsuhiro Kohada'" <kohada.t2@gmail.com>
Cc: <kohada.tetsuhiro@dc.mitsubishielectric.co.jp>,
	<mori.takahiro@ab.mitsubishielectric.co.jp>,
	<motai.hirotaka@aj.mitsubishielectric.co.jp>,
	"'Namjae Jeon'" <namjae.jeon@samsung.com>,
	<linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [RFC]PATCH] exfat: integrates dir-entry getting and validation
Date: Mon, 13 Jul 2020 11:19:29 +0900	[thread overview]
Message-ID: <4a6201d658bc$09e3fa80$1dabef80$@samsung.com> (raw)
In-Reply-To: <20200626060947.24709-1-kohada.t2@gmail.com>

> Add validation for num, bh and type on getting dir-entry.
> ('file' and 'stream-ext' dir-entries are pre-validated to ensure success)
> Renamed exfat_get_dentry_cached() to exfat_get_validated_dentry() due to a
> change in functionality.
> 
> Integrate type-validation with simplified.
> This will also recognize a dir-entry set that contains 'benign secondary'
> dir-entries.
> 
> And, rename TYPE_EXTEND to TYPE_NAME.
> 
> Suggested-by: Sungjong Seo <sj1557.seo@samsung.com>
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> ---
>  fs/exfat/dir.c      | 144 ++++++++++++++++++--------------------------
>  fs/exfat/exfat_fs.h |  15 +++--
>  fs/exfat/file.c     |   4 +-
>  fs/exfat/inode.c    |   6 +-
>  fs/exfat/namei.c    |   4 +-
>  5 files changed, 73 insertions(+), 100 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
> f4cea9a7fd02..e029e0986edc 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
[snip]
>   */
>  struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block
*sb,
> -		struct exfat_chain *p_dir, int entry, unsigned int type)
> +		struct exfat_chain *p_dir, int entry, int max_entries)
>  {
>  	int ret, i, num_bh;
> -	unsigned int off, byte_offset, clu = 0;
> +	unsigned int byte_offset, clu = 0;
>  	sector_t sec;
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	struct exfat_entry_set_cache *es;
>  	struct exfat_dentry *ep;
> -	int num_entries;
> -	enum exfat_validate_dentry_mode mode = ES_MODE_STARTED;
>  	struct buffer_head *bh;
> 
>  	if (p_dir->dir == DIR_DELETED) {
> @@ -844,13 +815,13 @@ struct exfat_entry_set_cache
> *exfat_get_dentry_set(struct super_block *sb,
>  		return NULL;
>  	es->sb = sb;
>  	es->modified = false;
> +	es->num_entries = 1;
> 
>  	/* byte offset in cluster */
>  	byte_offset = EXFAT_CLU_OFFSET(byte_offset, sbi);
> 
>  	/* byte offset in sector */
> -	off = EXFAT_BLK_OFFSET(byte_offset, sb);
> -	es->start_off = off;
> +	es->start_off = EXFAT_BLK_OFFSET(byte_offset, sb);
> 
>  	/* sector offset in cluster */
>  	sec = EXFAT_B_TO_BLK(byte_offset, sb); @@ -861,15 +832,12 @@ struct
> exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
>  		goto free_es;
>  	es->bh[es->num_bh++] = bh;
> 
> -	ep = exfat_get_dentry_cached(es, 0);
> -	if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
> +	ep = exfat_get_validated_dentry(es, 0, TYPE_FILE);
> +	if (!ep)
>  		goto free_es;
> +	es->num_entries = min(ep->dentry.file.num_ext + 1, max_entries);
> 
> -	num_entries = type == ES_ALL_ENTRIES ?
> -		ep->dentry.file.num_ext + 1 : type;
> -	es->num_entries = num_entries;
> -
> -	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE,
> sb);
> +	num_bh = EXFAT_B_TO_BLK_ROUND_UP(es->start_off  + es->num_entries *
> +DENTRY_SIZE, sb);
>  	for (i = 1; i < num_bh; i++) {
>  		/* get the next sector */
>  		if (exfat_is_last_sector_in_cluster(sbi, sec)) { @@ -889,11
> +857,13 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct
> super_block *sb,
>  	}
> 
>  	/* validiate cached dentries */
> -	for (i = 1; i < num_entries; i++) {
> -		ep = exfat_get_dentry_cached(es, i);
> -		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))

> +	for (i = 1; i < es->num_entries; i++) {
> +		if (!exfat_get_validated_dentry(es, i, TYPE_SECONDARY))
>  			goto free_es;
>  	}
> +	if (!exfat_get_validated_dentry(es, 1, TYPE_STREAM))
> +		goto free_es;

It looks better to move checking TYPE_STREAM above the for-loop.
And then for-loop should start from index 2.

BTW, do you think it is enough to check only TYPE_SECONDARY not TYPE NAME?
As you might know, FILE, STREAM and NAME entries must be consecutive in
order.

> +
>  	return es;
> 


  reply	other threads:[~2020-07-13  2:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200626061009epcas1p24585a6472e7103dc878bf9fc1d0f7d12@epcas1p2.samsung.com>
2020-06-26  6:09 ` [RFC]PATCH] exfat: integrates dir-entry getting and validation Tetsuhiro Kohada
2020-07-13  2:19   ` Sungjong Seo [this message]
2020-07-14  3:35     ` Kohada.Tetsuhiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='4a6201d658bc$09e3fa80$1dabef80$@samsung.com' \
    --to=sj1557.seo@samsung.com \
    --cc=kohada.t2@gmail.com \
    --cc=kohada.tetsuhiro@dc.mitsubishielectric.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mori.takahiro@ab.mitsubishielectric.co.jp \
    --cc=motai.hirotaka@aj.mitsubishielectric.co.jp \
    --cc=namjae.jeon@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox