Re: [Ext4 Secure Delete 7/7v4] ext4/jbd2: Secure Delete: Secure delete journal blocks

Allison Henderson <achender@xxxxxxxxxxxxxxxxxx> · Fri, 07 Oct 2011 17:06:42 -0700

On 10/07/2011 01:58 PM, Darrick J. Wong wrote:
On Fri, Oct 07, 2011 at 12:55:30PM -0700, Allison Henderson wrote:
On 10/07/2011 11:35 AM, Darrick J. Wong wrote:
On Fri, Oct 07, 2011 at 12:11:05AM -0700, Allison Henderson wrote:
This patch modifies both ext4 and jbd2 such that the journal
blocks which may contain file data, are securely deleted
after the files data blocks are deleted.

Because old journal blocks may contain file data, we need
a way to find those blocks again when it comes time to secure
delete the file.  This patch adds a new list to the journal
structure to keep track of which vfs blocks the journal blocks
contain.

After a truncate or a punch hole operation has completed, a
new function ext4_secure_delete_jblks is called that flushes
the journal, and then searches the list for any journal blocks
that were used to journal the blocks that were just removed.
The found journal blocks are then secure deleted.

Signed-off-by: Allison Henderson<achender@xxxxxxxxxxxxxxxxxx>
---
:100644 100644 0cba63b... fdf73b5... M	fs/ext4/ext4.h
:100644 100644 984fac2... 81df943... M	fs/ext4/extents.c
:100644 100644 bd1facd... 083c516... M	fs/ext4/inode.c
:100644 100644 eef6979... 2734e7b... M	fs/jbd2/commit.c
:100644 100644 f24df13... 6dd8af7... M	fs/jbd2/journal.c
:100644 100644 38f307b... 8b84b43... M	include/linux/jbd2.h
   fs/ext4/ext4.h       |    3 +
   fs/ext4/extents.c    |   12 +++++
   fs/ext4/inode.c      |  110 +++++++++++++++++++++++++++++++++++++++++++++++++
   fs/jbd2/commit.c     |    6 +++
   fs/jbd2/journal.c    |  112 ++++++++++++++++++++++++++++++++++++++++++++++++++
   include/linux/jbd2.h |   21 +++++++++
   6 files changed, 264 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 0cba63b..fdf73b5 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2230,6 +2230,9 @@ extern int ext4_secure_delete_lblks(struct inode *inode,
   			ext4_lblk_t first_block, unsigned long count);
   extern int ext4_secure_delete_pblks(struct inode *inode,
   			ext4_fsblk_t block, unsigned long count);
+extern int ext4_secure_delete_jblks(struct inode *inode,
+				ext4_lblk_t first_block, unsigned long count);
+

   /* move_extent.c */
   extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 984fac2..81df943 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4159,6 +4159,7 @@ int ext4_ext_punch_hole(struct file *file, loff_t offset, loff_t length)
   	struct super_block *sb = inode->i_sb;
   	struct ext4_ext_cache cache_ex;
   	ext4_lblk_t first_block, last_block, num_blocks, iblock, max_blocks;
+	ext4_lblk_t first_secrm_blk, last_secrm_blk, secrm_blk_count;
   	struct address_space *mapping = inode->i_mapping;
   	struct ext4_map_blocks map;
   	handle_t *handle;
@@ -4317,6 +4318,17 @@ out:
   	inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
   	ext4_mark_inode_dirty(handle, inode);
   	ext4_journal_stop(handle);
+
+	if (!err&&   EXT4_I(inode)->i_flags&   EXT4_SECRM_FL) {
+		first_secrm_blk = offset>>   EXT4_BLOCK_SIZE_BITS(sb);
+		last_secrm_blk = (offset + length + sb->s_blocksize - 1)>>
+			EXT4_BLOCK_SIZE_BITS(sb);
+		secrm_blk_count = last_secrm_blk - first_secrm_blk;
+
+		err = ext4_secure_delete_jblks(inode,
+		   first_secrm_blk, secrm_blk_count);
+	}
+
   	return err;
   }
   int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bd1facd..083c516 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -909,6 +909,110 @@ int ext4_secure_delete_lblks(struct inode *inode, ext4_lblk_t first_block,
   	return err;
   }

+/*
+ * ext4_secure_delete_jblks()
+ *
+ * Secure deletes the journal blocks that contain
+ * the specified logical blocks
+ *
+ * @inode: The files inode
+ * @first_block: Starting logical block
+ * @count: The number of blocks to secure delete
+ *         from the journal
+ *
+ * Returns 0 on sucess or negative on error
+ */
+int ext4_secure_delete_jblks(struct inode *inode,
+				ext4_lblk_t first_block, unsigned long count){
+
+	unsigned long long jbd2_pblk_start = 0;
+	unsigned long long jbd2_pblk_count = 0;
+	ext4_lblk_t last_block = 0;
+	struct list_head *tmp = NULL;
+	struct list_head *cur = NULL;
+	struct jbd2_blk_pair *b_pair = NULL;
+	int err = 0;
+	journal_t *journal = EXT4_JOURNAL(inode);
+
+	/* Do not allow last_block to wrap */
+	last_block = first_block + count;
+	if (last_block<   first_block)
+		last_block = EXT_MAX_BLOCKS;
+
+	/*
+	 * Force the journal to finnish up any pending transactions
+	 * before we start secure deleteing journal blocks
+	 */
+	err = ext4_force_commit((inode)->i_sb);
+	if (err)
+		return err;
+
+	spin_lock(&journal->j_pair_lock);
+
+	/* Loop over the journals blocks looking for our logical blocks */
+	list_for_each_safe(cur, tmp,&journal->blk_pairs) {
+		b_pair = list_entry(cur, struct jbd2_blk_pair, list);

Um.... I don't think ext4 should be accessing journal internals.  At a bare
minimum the stuff that mucks around with jbd2 ought to be in fs/jbd2 and
the ext4 parts stuffed in a wrapper in ext4_jbd2.[ch], since ocfs2 also uses
jbd2.

Ok, I can move the looping the jbd2 and set up a call back for doing the
actual secure deleting


I'm also wondering -- this logical<->   journal block mapping doesn't seem to be
committed to disk anywhere.  What happens if jbd2 crashes before we get to
zeroing journal blocks?  Specifically, would the journal recovery code know
that a given journal block also needs secure deletion?

Here's a counterproposal: What if ext4 told jbd2 which blocks need to be
securely deleted while ext4 is creating the transactions?  jbd2 could then set
a JBD2_FLAG_SECURE_DELETE flag in journal_block_tag_t.t_flags (the descriptor
block), which would tell the recovery and commit code that the associated
journal block needs secure deletion when processing is complete.  I _think_ you
could just extend the functions called by ext4_jbd2.c to take a flags
parameter.  Does this sound better?  Or even sane? :)

(Not sure if ocfs2 cares about secure delete at all.)

--D


Well actually I had initially started out with something that tried to
do the secure delete when the transactions had completed, but it didnt
work out because most of the time the transactions had already long
since completed before the secure delete flag even gets turned on. And

Ahh, I see, you're concerned that:

$ touch /mnt/afile		# A
$ sync
$ chattr +s /mnt/afile
$ rm -rf /mnt/afile

...doesn't secure delete the journal blocks associated with the inode creation
in step A, because one cannot create files with the secure deletion flag set.
I could think of a few ways out of this...

0) Simply accept that there's no way to create files with "secure delete" set;
therefore, the inode creation and dir entry creation will always be hanging out
in the journal.  This requires users to notice this little detail and get the
following details correct: if filenames are important, then they'll have to
create the file with a bogus name, chattr +s, create a hard link with the real
name, and then unlink the first bogus name. (ha ha....)

1) When creating a file, lie to the journal by always telling it to securely
delete the blocks associated with the file create.  If the user really wants
secure deletion, the next step after creating the file ought to be setting the
+s flag.  This has the advantage that even the generic "new inode" contents are
also protected by secure deletion, which #0 doesn't provide.  However, we'd
take a speed hit for a feature that isn't necessarily the common case.

2) Teach the journal to keep track of the pblocks and go back and retroactively
delete older copies, sort of like what your current patch does.

As a side note, we could probably discard journal blocks when the transaction
finishes committing and flushing.


Alrighty, you've given me a few more ideas to play with.  I think I may 
go back and do a little more prototyping and see what I can come up 
with.  Thx for the thorough review!!

Allison Henderson

then when we turn on the flag and start deleting, I have to get those
old journal blocks back.  So I think the new idea might have the same
problem.  But I think your right about needing to somehow commit it to
the disk, otherwise those old journal blocks will still be there after a
crash.



--D

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html





