Re: [PATCH 1/4] tune2fs: prevent changing UUID of fs with stable_inodes feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux