On Thu, Apr 08, 2021 at 10:51:09PM -0400, Theodore Ts'o wrote: > On Thu, Apr 08, 2021 at 05:02:07PM -0700, Darrick J. Wong wrote: > > > In the ideal world, sure, all or most of them would agree that they > > > *shouldn't* be storing any kind of PII at rest unencrypted, but they > > > can't be sure, and so from the perspective of keeping their audit and > > > I/T compliance committees happy, this requirement is desirable from a > > > "belt and suspenders" perspective. > > > > > > > This seems like a better fit for FITRIM than anything else. > > > > > > > > Ooohh. We sure do suck at APIs, don't we? FITRIM has no flags field, > > > > so we can't extend that. > > > > > > I don't have any serious objections to defining FITRIM2; OTOH, for > > > > Er, are we talking about the directory name wiping, or the journal > > discarding? > > Sorry, I was talking about journal wiping. The conflation is because > the reason why we want to wipe the journal is because of the directory > names in the journal, so the two are very much connected for our use > case, but yes, directory names in directories is very from directory > names in the journal. > > We don't actually need any kind of interface for wiping names in > directories, since it doesn't cost us anything to unconditionally wipe > the directory entries as opposed to just setting the inode number to > zero. > > > I didn't think it was any more difficult than changing xfs_removename to > > zero out the name and ftype fields at the same time it adds the whiteout > > to the dirent. But TBH I haven't thought through this too deeply. > > > > I /do/ think that if you ever want to add "secure" deletion to XFS, I'd > > want to do it by implementing FS_SECRM_FL for XFS, and not by adding > > more mount options. > > The original meaning of FS_SECRM_FL was that the data blocks would be > zero'ed --- when the inode was deleted. Sure, if discard is Good Enough(tm) for the journal, then we just treat this flag like "-o discard" was enabled for this file. Let the hardware do the "zeroing" in the background once we mark the extent as free. And if the hardware supports secure erase in place of discard, then even better. In the case of XFS, if we are to implement this directory entry zeroing then we actually need to discard the directory blocks. We may elide writeback of the directory block altogether if it is removed from the directory entirely between journal checkpoints. In that situation, we just write a whiteout for the block to the journal (we cancel the buffer) and we never actually write that buffer's contents to disk as it has been marked free by the journal commit. And, similarly short form directories aren't in blocks and can't be discarded, and we can elide inode writeback in the case where the inode clusters are freed. Hence zeroing dirents held inline in the inodes are not guaranteed to hit the disk, either. So we'd need to discard inode clusters as well. IOWs, we can do "rm -rf" of a directory with tens of thousands of entries, and the only thing that ends up hitting stable storage is a few hundred buffer invalidations in the journal. They remain unmodified in free space after the journal commit. This is why I said "good luck" to fixing XFS not to leak directory entries to disk. It's a pretty major undertaking to audit, fix and verify all the paths that remove directory entries to ensure that we do not leak dirent names anywhere. And I haven't even touched on PII in extended attributes :/ Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
