Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Namjae Jeon" <namjae.jeon@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>,
	"'Sungjong Seo'" <sj1557.seo@samsung.com>,
	<linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3] exfat: integrates dir-entry getting and validation
Date: Wed, 26 Aug 2020 16:32:04 +0900	[thread overview]
Message-ID: <004301d67b7a$ff0dcf50$fd296df0$@samsung.com> (raw)
In-Reply-To: <7d7ec460-b5ab-68da-658b-2104f393b4e8@gmail.com>

> Thank you for quick reply!
> 
> On 2020/08/26 13:19, Namjae Jeon wrote:
> >> On 2020/08/26 10:03, Namjae Jeon wrote:
> >>>> Second: Range validation and type validation should not be separated.
> >>>> When I started making this patch, I intended to add only range validation.
> >>>> However, after the caller gets the ep, the type validation follows.
> >>>> Get ep, null check of ep (= range verification), type verification is a series of procedures.
> >>>> There would be no reason to keep them independent anymore.
> >>>> Range and type validation is enforced when the caller uses ep.
> >>> You can add a validate flags as argument of exfat_get_dentry_set(), e.g. none, basic and strict.
> >>> none : only range validation.
> >>> basic : range + type validation.
> >>> strict : range + type + checksum and name length, etc.
> >>
> >> Currently, various types of verification will not be needed.
> >> Let's add it when we need it.
> >>>
> >>>>> -	/* 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))
> >>>>> +	ep = exfat_get_dentry_cached(es, ENTRY_STREAM);
> >>>>> +	if (!ep || ep->type != EXFAT_STREAM)
> >>>>> +		goto free_es;
> >>>>> +	es->de[ENTRY_STREAM] = ep;
> >>>>
> >>>> The value contained in stream-ext dir-entry should not be used
> >>>> before validating the EntrySet
> >> checksum.
> >>>> So I would insert EntrySet checksum validation here.
> >>>> In that case, the checksum verification loop would be followed by
> >>>> the TYPE_NAME verification loop, can you acceptable?
> >>> Yes. That would be great.
> >>
> >> OK.
> >> I'll add TYPE_NAME verification after checksum verification, in next patch.
> >> However, I think it is enough to validate TYPE_NAME when extracting name.
> >> Could you please tell me why you think you need TYPE_NAME validation here?
> > I've told you on previous mail. This function should return validated
> > dentry set after checking
> > file->stream->name in sequence.
> 
> Yes. I understand that the current implementation checks in that order.
> Sorry, my question was unclear.
> Why do you think you should leave the TYPE_NAME validation in this function?
> What kind of problem are you worried about if this function does not validate TYPE_NAME?
> (for preserve the current behavior?)
We have not checked the problem when it is removed because it was implemented
according to the specification from the beginning. And your v3 patch are
already checking the name entries as TYPE_SECONDARY. And it check them with
TYPE_NAME again in exfat_get_uniname_from_ext_entry(). If you check TYPE_NAME
with stream->name_len, We don't need to perform the loop for extracting
filename from the name entries if stream->name_len or name entry is invalid.

And I request to prove why we do not need to validate name entries in this
function calling from somewhere. So as I suggested earlier, You can make it
with an argument flags so that we skip the validation.
> 
> Don't worry, I will add TYPE_NAME verification to the v4 patch.
> I will post it later today.
Sound good.
> 
> BR
> ---
> Tetsuhiro Kohada <kohada.t2@gmail.com>


  reply	other threads:[~2020-08-26  7:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200806010249epcas1p18fd6e3febad305975b43e1b55b56bcae@epcas1p1.samsung.com>
2020-08-06  1:02 ` Tetsuhiro Kohada
2020-08-08 16:35   ` Sungjong Seo
2020-08-10  6:10   ` Namjae Jeon
2020-08-12 13:25     ` Tetsuhiro Kohada
2020-08-21  6:53       ` Namjae Jeon
2020-08-25  8:21         ` Tetsuhiro Kohada
2020-08-26  1:03           ` Namjae Jeon
2020-08-26  2:56             ` Tetsuhiro Kohada
2020-08-26  4:19               ` Namjae Jeon
2020-08-26  6:07                 ` Tetsuhiro Kohada
2020-08-26  7:32                   ` Namjae Jeon [this message]
2020-08-27  9:59                     ` Tetsuhiro Kohada

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='004301d67b7a$ff0dcf50$fd296df0$@samsung.com' \
    --to=namjae.jeon@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=sj1557.seo@samsung.com \
    --subject='RE: [PATCH v3] exfat: integrates dir-entry getting and validation' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).