Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200624023107.182118-1-gwendal@chromium.org> <20200624024043.GA844@sol.localdomain>
In-Reply-To: <20200624024043.GA844@sol.localdomain>
From:   Gwendal Grignou <gwendal@chromium.org>
Date:   Wed, 24 Jun 2020 00:06:22 -0700
Message-ID: <CAPUE2uv0XXv40quqbKmdktgCD18DnSWh=Ekeiq2tAZOfmGmjGw@mail.gmail.com>
Subject: Re: [PATCH] tune2fs: allow remove VERITY
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Sarthak Kukreti <sarthakkukreti@chromium.org>, tytso@mit.edu,
        linux-ext4@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

On Tue, Jun 23, 2020 at 7:40 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jun 23, 2020 at 07:31:07PM -0700, Gwendal Grignou wrote:
> > Allow verity flag to be removed from the susperblock:
> > Tests:
> > - check the signed file is readable by older kernel after flag
> > is removed. EXT4_VERITY_FL replaces EXT4_EXT_MIGRATE that has been
> > removed in 2009.
> > - when a new kernel is reinstalled, check reenabling verity flag
> > allow signature to be verified (fsverity measure ...).
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> > ---
> >  misc/tune2fs.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > index 314cc0d0..724b8014 100644
> > --- a/misc/tune2fs.c
> > +++ b/misc/tune2fs.c
> > @@ -198,7 +198,8 @@ static __u32 clear_ok_features[3] = {
> >               EXT4_FEATURE_RO_COMPAT_QUOTA |
> >               EXT4_FEATURE_RO_COMPAT_PROJECT |
> >               EXT4_FEATURE_RO_COMPAT_METADATA_CSUM |
> > -             EXT4_FEATURE_RO_COMPAT_READONLY
> > +             EXT4_FEATURE_RO_COMPAT_READONLY |
> > +             EXT4_FEATURE_RO_COMPAT_VERITY
> >  };
> >
>
> tune2fs doesn't allow removing features like encrypt, casefold, verity, extents,
> and ea_inode because it doesn't know whether there are any inodes on the
> filesystem that are using these features.  These features can't be removed if
> there are any inodes using them.
The verity case is slightly different though: beside metadata,
encrypted files are useless.
In the case of fs-verity, the file is still readable, its size is
correct. Using debugfs, I checked the merkel tree blocks appended at
the end of the file are still mapped to the file inode, they are
marked as free when the file is removed.
Are you concerned about filesystem corruption? When I re-enable the
features and load a kernel that supports fs-verity, the protected
files are signed and read-only as expected.

>
> There was recently a suggestion to make tune2fs scan the inode table to
> determine whether it is safe to remove these flags; see
> https://lkml.kernel.org/linux-ext4/C0761869-5FCD-4CC7-9635-96C18744A0F8@dilger.ca
> and
> https://lkml.kernel.org/linux-ext4/20200407053213.GC102437@sol.localdomain
The intent is to be able to rollback to an older kernel. Given the
file system will contain files protected by fs-verity, tune2fs will
not be able to remove the feature.
>
> I think you'd need to implement that in order for clearing verity to be safe.
>
> Note that misc/tune2fs.8.in would also need to be updated to remove the sentence
> that says that clearing the verity feature is unsuppported.
Will do.

Gwendal.
>
> - Eric