linux-kernel - Re: erofs: Question on unused fields in on-disk structs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Aug 2019 10:33:01 +0200
From:   Richard Weinberger <richard.weinberger@...il.com>
To:     Gao Xiang <hsiangkao@....com>
Cc:     Richard Weinberger <richard@....at>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-erofs@...ts.ozlabs.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: erofs: Question on unused fields in on-disk structs

On Thu, Aug 22, 2019 at 12:03 AM Gao Xiang <hsiangkao@....com> wrote:
>
> Hi Richard,
>
> On Wed, Aug 21, 2019 at 11:37:30PM +0200, Richard Weinberger wrote:
> > Gao Xiang,
> >
> > On Mon, Aug 19, 2019 at 10:45 PM Gao Xiang via Linux-erofs
> > <linux-erofs@...ts.ozlabs.org> wrote:
> > > > struct erofs_super_block has "checksum" and "features" fields,
> > > > but they are not used in the source.
> > > > What is the plan for these?
> > >
> > > Yes, both will be used laterly (features is used for compatible
> > > features, we already have some incompatible features in 5.3).
> >
> > Good. :-)
> > I suggest to check the fields being 0 right now.
> > Otherwise you are in danger that they get burned if an mkfs.erofs does not
> > initialize the fields.
>
> Sorry... I cannot get the point...

Sorry for being unclear, let me explain in more detail.

> super block chksum could be a compatible feature right? which means
> new kernel can support it (maybe we can add a warning if such image
> doesn't have a chksum then when mounting) but old kernel doesn't
> care it.

Yes. But you need some why to indicate that the chksum field is now
valid and must be used.

The features field can be used for that, but you don't use it right now.
I recommend to check it for being 0, 0 means then "no features".
If somebody creates in future a erofs with more features this code
can refuse to mount because it does not support these features.

But be very sure that existing erofs filesystems actually have this field
set to 0 or something other which is always the same.
Otherwise you cannot use the field anymore because it could be anything.
A common bug is that the mkfs program keeps such unused fields
uninitialized and then it can be a more or less random value without
notice.

> Or maybe you mean these reserved fields? I have no idea all other
> filesystems check these fields to 0 or not... But I think it should
> be used with some other flag is set rather than directly use, right?

Basically you want a way to know when a field shall be used and when not.
Most filesystems have version/feature fields. Often multiple to denote different
levels of compatibility.

-- 
Thanks,
//richard