linux-kernel - Re: [PATCH] f2fs: introduce on-disk layout version checking functionality

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]

Message-ID: <1464137630.2680.17.camel@slavad-ubuntu-14.04>
Date:	Tue, 24 May 2016 17:53:50 -0700
From:	Viacheslav Dubeyko <slava@...eyko.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Jaegeuk Kim <jaegeuk@...nel.org>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net,
	Vyacheslav.Dubeyko@...t.com, Cyril.Guyot@...t.com,
	Adam.Manzanares@...t.com, Damien.LeMoal@...t.com
Subject: Re: [PATCH] f2fs: introduce on-disk layout version checking
 functionality

On Tue, 2016-05-24 at 01:52 -0700, Christoph Hellwig wrote:
> On Mon, May 23, 2016 at 01:08:05PM -0700, Viacheslav Dubeyko wrote:
> > I think that it's some confusion. I didn't introduce any new fields in
> > struct f2fs_super_block. The "major_ver" and "minor_ver" fields exist in
> > F2FS superblock from the beginning of this file system implementation.
> > The content of these two fields are defined during mkfs phase. The
> > f2fs_format.c contains such code in f2fs_prepare_super_block():
> 
> They exists, but the kernel so far never checked them, and despite
> that the feature checking works fine worth other f2fs features.
> 
> > Current version in VERSION file is 1.6.1. So, historically F2FS is using
> > version of on-disk layout. The suggested patch simply introduces the
> > threshold value F2FS_MAX_SUPP_MAJOR_VERSION with the purpose to refuse
> > the mount operation for the case of unsupported version of on-disk
> > layout.
> 
> While I've never seen an actual piece of documentation for the fields it
> seems so far they just document the version of mkfs used to create
> the file system.  Suddenly overloading them with semantics is just
> going to create problems.
> 

The best way not to create a problem is to do nothing.

The F2FS superblock has "major_ver" and "minor_ver" fields. This
metadata structure is stored into F2FS volume. So, this two fields
define the on-disk layout's version. We are trying to change the on-disk
layout. It means that we need to increase the on-disk layout's version
number and to check the version number, namely. What's wrong with my
logic?

> > First of all, it needs to distinguish two different points. First point,
> > we need to increase the on-disk layout version because we are going to
> > change on-disk layout in the way that old (current) driver will not
> > support.
> 
> That's exactly what most file systems use feature flags for.

Frankly speaking, support of 16TB+ volumes is not a "feature" but simple
bug fix. Because this issue was created during metadata structure
definitions. And we are trying to fix this issue right now. And this
issue is on-disk layout related issue. So, the key point here is not a
feature flag but the on-disk layout's version, from my point of view.

Thanks,
Vyacheslav Dubeyko.