> On Apr 6, 2020, at 11:32 PM, Eric Biggers <ebiggers@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: >>> >>> From: Eric Biggers <ebiggers@xxxxxxxxxx> >>> >>> 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
Attachment:
signature.asc
Description: Message signed with OpenPGP
