[PATCH 02/19] mkfs: sanitise ftype parameter values.

[Jan Tulak]

On Tue, Mar 29, 2016 at 6:17 PM, Eric Sandeen <sandeen at sandeen.net> wrote:

>
>
> On 3/29/16 11:11 AM, Jan Tulak wrote:
> > On Thu, Mar 24, 2016 at 5:33 PM, Eric Sandeen <sandeen at sandeen.net
> <mailto:sandeen at sandeen.net>>wrote:
> >
> >
> >
> >     On 3/24/16 6:15 AM, jtulak at redhat.com <mailto:jtulak at redhat.com>
> wrote:
> >     > From: Dave Chinner <dchinner at redhat.com <mailto:
> dchinner at redhat.com>>
> >     >
> >     > Because passing "-n ftype=2" should fail.
> >
> >     but passing crc=1 ftype=1 shouldn't fail, should it?
> >     Seems like it will here.
> >
> >
> > ​From man page:
> > When  CRCs are enabled via -m crc=1, the ftype functionality is always
> enabled. This feature can not be
> > ​
> > turned off for such filesystem configurations.​
> >
> >
> > ​So I think it should not be possible to enter both crc and ftype at
> > the same time - which is the current behaviour. It feels strange a
> > bit to allow ftype=1 (which does nothing with crc=1), but fail on
> > ftype=0​
>
> My point is that -m crc=1 -d ftype=1 simply restates the defaults.
> Why should that combination fail?
>
> And -m crc=0 -d ftype=0 is also perfectly acceptable.
>
> In fact, -m crc=1 -d ftype=0 is the only one of the 4 combinations
> which is not ok, but AFAICT your patch fails the other 3 as well.
>

​I removed the entire crc check - there is another one later in the code,
so this was 1) redundant, 2) wrong, because it depended on whether -m crc=0
was before or after -n ftype (if after, default crc value was used for the
check).

However, it causes lots of conflicts on the following patches, so I will
send it later, once there are more changes...

Cheers, and thanks for catching it.

Jan

-- 
Jan Tulak
jtulak at redhat.com / jan at tulak.me
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://oss.sgi.com/pipermail/xfs/attachments/20160329/070f3563/attachment.html>