linux-ext4 - Re: [PATCH 1/4] tune2fs: prevent changing UUID of fs with stable

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: <74B95427-9FB1-4DF8-BE75-CE099EA3A9A3@dilger.ca>
Date:   Tue, 7 Apr 2020 10:18:55 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     linux-ext4 <linux-ext4@...r.kernel.org>,
        linux-fscrypt@...r.kernel.org
Subject: Re: [PATCH 1/4] tune2fs: prevent changing UUID of fs with
 stable_inodes feature


> On Apr 6, 2020, at 11:32 PM, Eric Biggers <ebiggers@...nel.org> wrote:
> 
> On Wed, Apr 01, 2020 at 08:19:38PM -0600, Andreas Dilger wrote:
>> On Apr 1, 2020, at 2:32 PM, Eric Biggers <ebiggers@...nel.org> wrote:
>>> 
>>> From: Eric Biggers <ebiggers@...gle.com>
>>> 
>>> The stable_inodes feature is intended to indicate that it's safe to use
>>> IV_INO_LBLK_64 encryption policies, where the encryption depends on the
>>> inode numbers and thus filesystem shrinking is not allowed.  However
>>> since inode numbers are not unique across filesystems, the encryption
>>> also depends on the filesystem UUID, and I missed that there is a
>>> supported way to change the filesystem UUID (tune2fs -U).
>>> 
>>> So, make 'tune2fs -U' report an error if stable_inodes is set.
>>> 
>>> We could add a separate stable_uuid feature flag, but it seems unlikely
>>> it would be useful enough on its own to warrant another flag.
>> 
>> What about having tune2fs walk the inode table checking for any inodes that
>> have this flag, and only refusing to clear the flag if it finds any?  That
>> takes some time on very large filesystems, but since inode table reading is
>> linear it is reasonable on most filesystems.
> 
> I assume you meant to make this comment on patch 2,
> "tune2fs: prevent stable_inodes feature from being cleared"?
> 
> It's a good suggestion, but it also applies equally to the encrypt, verity,
> extents, and ea_inode features.  Currently tune2fs can't clear any of these,
> since any inode might be using them.
> 
> Note that it would actually be slightly harder to implement your suggestion for
> stable_inodes than those four existing features, since clearing stable_inodes
> would require reading xattrs rather than just the inode flags.
> 
> So if I have time, I can certainly look into allowing tune2fs to clear the
> encrypt, verity, extents, stable_inodes, and ea_inode features, by doing an
> inode table scan to verify that it's safe.  IMO it doesn't make sense to hold up
> this patch on it, though.  This patch just makes stable_inodes work like other
> ext4 features.

Sure, I'm OK with this patch, since it avoids accidental breakage.

One question though - for the data checksums it uses s_checksum_seed to generate
checksums, rather than directly using the UUID itself, so that it *is* possible
to change the filesystem UUID after metadata_csum is in use, without the need
to rewrite all of the checksums in the filesystem.  Could the same be done for
stable_inode?

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)