Hi all,
Sorry for the delay in getting this next version out.
I had some tasks to take care of, and now I'm picking up my
secure delete work again. I'm still not quite done yet,
but a lot has changed and I wanted to update people so that
we have an idea of where its going. Currently the patch
deals with data blocks, meta blocks, directory entries,
journal blocks, and also provides an option for secure
deleting with random data instead of just zeros.
I'm also planning on adding some more patches to
deal with inodes and also a mount option that turns
on secure delete by default. Im still not quite done
debugging, but Im just sending it out early to get
some more eyes on it. Feed back appreciated! :)
v3->v4
Added a new file attribute flag EXT4_SECRM_RANDOM_FL
This flag causes the secure delete operations to over write
blocks with random data instead of zeros.
New function ext4_secure_delete_lblks added to walk
data blocks and secure delete them before any blocks
are removed.
Meta blocks are secure deleted before they are
released
New function added to identify holes in ind files.
Used by ext4_secure_delete_lblks to skip over holes
during secure delete.
Added another list in the journal structure to track
journal blocks so that they can be secure deleted later.
Added new ext4_secure_delete_jblks that secure deletes
journal blocks that were used to journal the specified
logical blocks
Allison Henderson (7):
ext4: Secure Delete: Add new EXT4_SECRM_RANDOM_FL flag
ext4: Secure Delete: Add ext4_ind_hole_lookup function
ext4: Secure Delete: Add secure delete functions
ext4: Secure Delete: Secure delete file data
ext4: Secure Delete: Secure delete directory entry
ext4: Secure Delete: Secure delete meta data blocks
ext4/jbd2: Secure Delete: Secure delete journal blocks
fs/ext4/ext4.h | 28 +++-
fs/ext4/ext4_extents.h | 2 +
fs/ext4/extents.c | 21 +++-
fs/ext4/indirect.c | 2 +-
fs/ext4/inode.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/mballoc.c | 8 +
fs/ext4/namei.c | 64 +++++++-
fs/jbd2/commit.c | 6 +
fs/jbd2/journal.c | 112 ++++++++++++++
include/linux/jbd2.h | 21 +++
10 files changed, 642 insertions(+), 13 deletions(-)
This patch modifies ext4_free_blocks to securely delete meta blocks
when needed
Signed-off-by: Allison Henderson <[email protected]>
---
:100644 100644 17a5a57... d25bb4d... M fs/ext4/mballoc.c
fs/ext4/mballoc.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 17a5a57..d25bb4d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4607,6 +4607,14 @@ do_more:
if (err)
goto error_return;
+ if ((flags & EXT4_FREE_BLOCKS_METADATA) &&
+ EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
+
+ err = ext4_secure_delete_pblks(inode, block, count);
+ if (err)
+ goto error_return;
+ }
+
if ((flags & EXT4_FREE_BLOCKS_METADATA) && ext4_handle_valid(handle)) {
struct ext4_free_data *new_entry;
/*
--
1.7.1
This patch zeros or randomizes a files directory entry when a file
with the EXT4_SECRM_FL attribute flag is deleted or renamed. A new
flag parameter has been added to the ext4_delete_entry routine,
that will cause the entry to be securely zeroed or randomized and
then flushed to the disk.
Signed-off-by: Allison Henderson <[email protected]>
---
v1->v2
Removed new inode parameter in ext4_delete_entry and replaced
with a new flag for ext4_delete_entry
:100644 100644 34f82a1... 0cba63b... M fs/ext4/ext4.h
:100644 100644 f8068c7... b3479c6... M fs/ext4/namei.c
fs/ext4/ext4.h | 6 +++++
fs/ext4/namei.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 34f82a1..0cba63b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -532,6 +532,12 @@ struct ext4_new_group_data {
#define EXT4_FREE_BLOCKS_NO_QUOT_UPDATE 0x0008
/*
+ * Flags used by ext4_delete_entry
+ */
+#define EXT4_DEL_ENTRY_ZERO 0x0001
+#define EXT4_DEL_ENTRY_RAND 0x0002
+
+/*
* ioctl commands
*/
#define EXT4_IOC_GETFLAGS FS_IOC_GETFLAGS
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index f8068c7..b3479c6 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -34,6 +34,7 @@
#include <linux/quotaops.h>
#include <linux/buffer_head.h>
#include <linux/bio.h>
+#include <linux/random.h>
#include "ext4.h"
#include "ext4_jbd2.h"
@@ -1639,9 +1640,11 @@ cleanup:
static int ext4_delete_entry(handle_t *handle,
struct inode *dir,
struct ext4_dir_entry_2 *de_del,
- struct buffer_head *bh)
+ struct buffer_head *bh,
+ int flags)
{
struct ext4_dir_entry_2 *de, *pde;
+ struct ext4_super_block *es = EXT4_SB(dir->i_sb)->s_es;
unsigned int blocksize = dir->i_sb->s_blocksize;
int i, err;
@@ -1669,7 +1672,38 @@ static int ext4_delete_entry(handle_t *handle,
de->inode = 0;
dir->i_version++;
BUFFER_TRACE(bh, "call ext4_handle_dirty_metadata");
- err = ext4_handle_dirty_metadata(handle, dir, bh);
+
+ /*
+ * If the secure remove flag is on, zero
+ * or randomize the entry and write it out
+ * to the disk
+ */
+ if (flags & EXT4_DEL_ENTRY_ZERO) {
+ memset(de->name, 0x00, de->name_len);
+ de->file_type = 0;
+ } else if (flags & EXT4_DEL_ENTRY_RAND) {
+ get_random_bytes(de->name, de->name_len);
+ get_random_bytes(&(de->file_type),
+ sizeof(de->file_type));
+ }
+
+ if (flags & EXT4_DEL_ENTRY_ZERO ||
+ flags & EXT4_DEL_ENTRY_RAND) {
+
+ set_buffer_dirty(bh);
+ sync_dirty_buffer(bh);
+ if (buffer_req(bh) && !buffer_uptodate(bh)) {
+ es->s_last_error_block =
+ cpu_to_le64(bh->b_blocknr);
+ ext4_error_inode(dir, __func__,
+ __LINE__, bh->b_blocknr,
+ "IO error syncing itable block");
+ err = -EIO;
+ }
+ } else
+ err = ext4_handle_dirty_metadata(handle,
+ dir, bh);
+
if (unlikely(err)) {
ext4_std_error(dir->i_sb, err);
return err;
@@ -2151,7 +2185,7 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
if (!empty_dir(inode))
goto end_rmdir;
- retval = ext4_delete_entry(handle, dir, de, bh);
+ retval = ext4_delete_entry(handle, dir, de, bh, 0);
if (retval)
goto end_rmdir;
if (!EXT4_DIR_LINK_EMPTY(inode))
@@ -2179,7 +2213,7 @@ end_rmdir:
static int ext4_unlink(struct inode *dir, struct dentry *dentry)
{
- int retval;
+ int retval, del_entry_flags;
struct inode *inode;
struct buffer_head *bh;
struct ext4_dir_entry_2 *de;
@@ -2204,6 +2238,13 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
goto end_unlink;
inode = dentry->d_inode;
+ del_entry_flags = 0;
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_RANDOM_FL)
+ del_entry_flags = EXT4_DEL_ENTRY_RAND;
+ else
+ del_entry_flags = EXT4_DEL_ENTRY_ZERO;
+ }
retval = -EIO;
if (le32_to_cpu(de->inode) != inode->i_ino)
@@ -2215,7 +2256,7 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
inode->i_ino, inode->i_nlink);
inode->i_nlink = 1;
}
- retval = ext4_delete_entry(handle, dir, de, bh);
+ retval = ext4_delete_entry(handle, dir, de, bh, del_entry_flags);
if (retval)
goto end_unlink;
dir->i_ctime = dir->i_mtime = ext4_current_time(dir);
@@ -2395,7 +2436,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *old_inode, *new_inode;
struct buffer_head *old_bh, *new_bh, *dir_bh;
struct ext4_dir_entry_2 *old_de, *new_de;
- int retval, force_da_alloc = 0;
+ int retval, del_entry_flags, force_da_alloc = 0;
dquot_initialize(old_dir);
dquot_initialize(new_dir);
@@ -2494,11 +2535,18 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
/*
* ok, that's it
*/
+ del_entry_flags = 0;
+ if (EXT4_I(old_inode)->i_flags & EXT4_SECRM_FL) {
+ if (EXT4_I(old_inode)->i_flags & EXT4_SECRM_RANDOM_FL)
+ del_entry_flags = EXT4_DEL_ENTRY_RAND;
+ else
+ del_entry_flags = EXT4_DEL_ENTRY_ZERO;
+ }
if (le32_to_cpu(old_de->inode) != old_inode->i_ino ||
old_de->name_len != old_dentry->d_name.len ||
strncmp(old_de->name, old_dentry->d_name.name, old_de->name_len) ||
(retval = ext4_delete_entry(handle, old_dir,
- old_de, old_bh)) == -ENOENT) {
+ old_de, old_bh, del_entry_flags)) == -ENOENT) {
/* old_de could have moved from under us during htree split, so
* make sure that we are deleting the right entry. We might
* also be pointing to a stale entry in the unused part of
@@ -2509,7 +2557,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
old_bh2 = ext4_find_entry(old_dir, &old_dentry->d_name, &old_de2);
if (old_bh2) {
retval = ext4_delete_entry(handle, old_dir,
- old_de2, old_bh2);
+ old_de2, old_bh2, del_entry_flags);
brelse(old_bh2);
}
}
--
1.7.1
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 <[email protected]>
---
: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);
+ if ((b_pair->vfs_inode == inode) &&
+ (b_pair->vfs_lblk >= first_block) &&
+ (b_pair->vfs_lblk < last_block)) {
+
+ /*
+ * It is likely that the journal blocks will be
+ * consecutive, so lets try to secure delete them
+ * in ranges
+ */
+ if (jbd2_pblk_count == 0) {
+ /*
+ * If there are no blocks in our range,
+ * then this one will be the first
+ */
+ jbd2_pblk_start = b_pair->jbd2_pblk;
+ jbd2_pblk_count = 1;
+ } else if ((b_pair->jbd2_pblk) ==
+ (jbd2_pblk_start + jbd2_pblk_count)) {
+ /*
+ * If this journal block is physically
+ * consecutive, then just increase the range
+ */
+ jbd2_pblk_count++;
+ } else if ((b_pair->jbd2_pblk) ==
+ (jbd2_pblk_start - 1) &&
+ jbd2_pblk_start > 0) {
+ /*
+ * If this journal block is physically
+ * consecutive (from the start of the
+ * range), just increase the range from the
+ * other end.
+ */
+ jbd2_pblk_count++;
+ jbd2_pblk_start--;
+ } else {
+ /*
+ * If the block was not consecutive, secure
+ * delete the range, and restart the current
+ * range
+ */
+
+ err = ext4_secure_delete_pblks(journal->j_inode,
+ jbd2_pblk_start, jbd2_pblk_count);
+ if (err)
+ goto out;
+ jbd2_pblk_start = b_pair->jbd2_pblk;
+ jbd2_pblk_count = 1;
+ }
+ }
+ }
+
+ /* Secure delete any blocks still in our range */
+ if (jbd2_pblk_count > 0)
+ err = ext4_secure_delete_pblks(journal->j_inode,
+ jbd2_pblk_start, jbd2_pblk_count);
+
+out:
+ spin_unlock(&journal->j_pair_lock);
+ return err;
+}
+
struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
ext4_lblk_t block, int create, int *err)
{
@@ -3447,6 +3551,12 @@ void ext4_truncate(struct inode *inode)
else
ext4_ind_truncate(inode);
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
+ err = ext4_secure_delete_jblks(inode,
+ inode->i_size >> EXT4_BLOCK_SIZE_BITS(inode->i_sb),
+ EXT_MAX_BLOCKS);
+ }
+
trace_ext4_truncate_exit(inode);
}
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index eef6979..2734e7b 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -593,6 +593,12 @@ void jbd2_journal_commit_transaction(journal_t *journal)
jbd2_journal_abort(journal, flags);
continue;
}
+
+ err = jbd2_record_pair(journal, jh2bh(jh), jh2bh(new_jh));
+ if (err) {
+ jbd2_journal_abort(journal, flags);
+ continue;
+ }
set_bit(BH_JWrite, &jh2bh(new_jh)->b_state);
wbuf[bufs++] = jh2bh(new_jh);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f24df13..6dd8af7 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -248,6 +248,100 @@ static void journal_kill_thread(journal_t *journal)
}
write_unlock(&journal->j_state_lock);
}
+/*
+ * jbd2_record_pair()
+ * Updates the journals list of block pairs to contain an new pair of journal
+ * to vfs blocks. This list is an in memory record of the journals blocks to
+ * help facilitate finding out what vfs blocks the journal blocks contain.
+ * Secure delete uses this list to clear out stale journal blocks that may
+ * still contain file data that needs to be secure deleted. This list is updated
+ * when a vfs data block is written to the journal or when a descriptor block
+ * is written to the journal.
+ *
+ * journal: The journal to update
+ * vfs_bh: The vfs buffer_head that the jbd2_bh is journaling. This
+ * may be NULL if jbd2_bh does not represent a vfs block.
+ * jbd2_bh: The jbd2 buffer_head that has just been allocated for some
+ * use in the journal. jbd2_bh may not be NULL.
+ *
+ * Returns 0 on sucess or negative on failure
+ */
+int jbd2_record_pair(journal_t *journal,
+ struct buffer_head *vfs_bh,
+ struct buffer_head *jbd2_bh){
+
+ struct page *page;
+ struct buffer_head *bh;
+ struct buffer_head *head;
+ pgoff_t index;
+ struct jbd2_blk_pair *b_pair = NULL;
+ struct list_head *cur;
+ unsigned long long vfs_lblk = 0;
+ unsigned long long vfs_pblk = 0;
+ struct inode *vfs_inode = NULL;
+
+ /* If we have the vfs bh, figure out the logical block offset */
+ if (vfs_bh) {
+ page = vfs_bh->b_page;
+ vfs_inode = page->mapping->host;
+ vfs_pblk = vfs_bh->b_blocknr;
+ index = page->index;
+
+ /* Find the block offset of the page */
+ vfs_lblk = index <<
+ (PAGE_CACHE_SHIFT - vfs_inode->i_sb->s_blocksize_bits);
+
+ /* Then add in the block offset of the bh in the page */
+ bh = page_buffers(page);
+ head = bh;
+ do {
+ if (bh == vfs_bh)
+ break;
+ bh = bh->b_this_page;
+ vfs_lblk++;
+ } while (bh != head);
+ }
+
+ spin_lock(&journal->j_pair_lock);
+ /*
+ * Add the pair to the list. If there is already a pair
+ * for this journal block, just update it with the new vfs
+ * block info
+ */
+ list_for_each(cur, &journal->blk_pairs) {
+ b_pair = list_entry(cur, struct jbd2_blk_pair, list);
+ if (b_pair->jbd2_pblk == jbd2_bh->b_blocknr) {
+ b_pair->vfs_inode = vfs_inode;
+ b_pair->vfs_pblk = vfs_pblk;
+ b_pair->vfs_lblk = vfs_lblk;
+ b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
+
+ break;
+ } else
+ b_pair = NULL;
+ }
+
+ /*
+ * If the journal block was not found in the list,
+ * add a new pair to the list
+ */
+ if (!b_pair) {
+ b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS);
+ if (!b_pair) {
+ spin_unlock(&journal->j_pair_lock);
+ return -ENOMEM;
+ }
+ b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
+ b_pair->vfs_inode = vfs_inode;
+ b_pair->vfs_pblk = vfs_pblk;
+ b_pair->vfs_lblk = vfs_lblk;
+
+ list_add(&b_pair->list, &journal->blk_pairs);
+ }
+
+ spin_unlock(&journal->j_pair_lock);
+ return 0;
+}
/*
* jbd2_journal_write_metadata_buffer: write a metadata buffer to the journal.
@@ -739,7 +833,13 @@ struct journal_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
lock_buffer(bh);
memset(bh->b_data, 0, journal->j_blocksize);
set_buffer_uptodate(bh);
+ err = jbd2_record_pair(journal, NULL, bh);
+
unlock_buffer(bh);
+
+ if (err)
+ return NULL;
+
BUFFER_TRACE(bh, "return this buffer");
return jbd2_journal_add_journal_head(bh);
}
@@ -895,9 +995,11 @@ static journal_t * journal_init_common (void)
init_waitqueue_head(&journal->j_wait_commit);
init_waitqueue_head(&journal->j_wait_updates);
mutex_init(&journal->j_barrier);
+ INIT_LIST_HEAD(&journal->blk_pairs);
mutex_init(&journal->j_checkpoint_mutex);
spin_lock_init(&journal->j_revoke_lock);
spin_lock_init(&journal->j_list_lock);
+ spin_lock_init(&journal->j_pair_lock);
rwlock_init(&journal->j_state_lock);
journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
@@ -1361,6 +1463,8 @@ recovery_error:
int jbd2_journal_destroy(journal_t *journal)
{
int err = 0;
+ struct list_head *cur;
+ struct jbd2_blk_pair *b_pair;
/* Wait for the commit thread to wake up and die. */
journal_kill_thread(journal);
@@ -1405,6 +1509,14 @@ int jbd2_journal_destroy(journal_t *journal)
iput(journal->j_inode);
if (journal->j_revoke)
jbd2_journal_destroy_revoke(journal);
+
+ spin_lock(&journal->j_pair_lock);
+ list_for_each_prev(cur, &journal->blk_pairs) {
+ b_pair = list_entry(cur, struct jbd2_blk_pair, list);
+ kfree(b_pair);
+ }
+ spin_unlock(&journal->j_pair_lock);
+
kfree(journal->j_wbuf);
kfree(journal);
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 38f307b..8b84b43 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -698,6 +698,19 @@ jbd2_time_diff(unsigned long start, unsigned long end)
#define JBD2_NR_BATCH 64
+
+/*
+ * List for keeping track of which vfs
+ * block a journal block contians.
+ */
+struct jbd2_blk_pair {
+ struct inode *vfs_inode;
+ unsigned long long vfs_pblk;
+ unsigned long long vfs_lblk;
+ unsigned long long jbd2_pblk;
+ struct list_head list;
+};
+
/**
* struct journal_s - The journal_s type is the concrete type associated with
* journal_t.
@@ -1002,6 +1015,9 @@ struct journal_s
* superblock pointer here
*/
void *j_private;
+
+ spinlock_t j_pair_lock;
+ struct list_head blk_pairs;
};
/*
@@ -1080,6 +1096,11 @@ jbd2_journal_write_metadata_buffer(transaction_t *transaction,
struct journal_head **jh_out,
unsigned long long blocknr);
+extern int jbd2_record_pair(journal_t *journal,
+ struct buffer_head *vfs_bh,
+ struct buffer_head *jbd2_bh);
+
+
/* Transaction locking */
extern void __wait_on_journal (journal_t *);
--
1.7.1
This patch adds a new ext4_ind_hole_lookup function that
will be used to identify and skip over holes during a
secure delete operation
Signed-off-by: Allison Henderson <[email protected]>
---
:100644 100644 db54ce4... 5c9f88c... M fs/ext4/ext4.h
:100644 100644 0962642... 2fe9dfd... M fs/ext4/indirect.c
:100644 100644 c4da98a... 9dc8c14... M fs/ext4/inode.c
fs/ext4/ext4.h | 5 +++
fs/ext4/indirect.c | 2 +-
fs/ext4/inode.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index db54ce4..5c9f88c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2241,6 +2241,11 @@ extern int ext4_bio_write_page(struct ext4_io_submit *io,
/* mmp.c */
extern int ext4_multi_mount_protect(struct super_block *, ext4_fsblk_t);
+/* indirects.c */
+extern int ext4_block_to_path(struct inode *inode,
+ ext4_lblk_t i_block,
+ ext4_lblk_t offsets[4], int *boundary);
+
/* BH_Uninit flag: blocks are allocated but uninitialized on disk */
enum ext4_state_bits {
BH_Uninit /* blocks are allocated but uninitialized on disk */
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 0962642..2fe9dfd 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -69,7 +69,7 @@ static inline void add_chain(Indirect *p, struct buffer_head *bh, __le32 *v)
* get there at all.
*/
-static int ext4_block_to_path(struct inode *inode,
+int ext4_block_to_path(struct inode *inode,
ext4_lblk_t i_block,
ext4_lblk_t offsets[4], int *boundary)
{
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c4da98a..9dc8c14 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -640,6 +640,79 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
return bh;
}
+/*
+ * ext4_in_hole_lookup
+ *
+ * Returns the size of the hole located at the given block up to
+ * the first boundry, or 0 if there is no hole. Returns negative on err.
+ */
+static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
+{
+ struct super_block *sb = inode->i_sb;
+ struct buffer_head *bh;
+ ext4_lblk_t offsets[4];
+ ext4_lblk_t offset;
+ __le32 *children;
+ __le32 blk;
+ int blocks_to_boundary = 0;
+ int depth = 0;
+ int hole_size = 0;
+ unsigned int i, j;
+
+ depth = ext4_block_to_path(inode, block, offsets, &blocks_to_boundary);
+
+ if (depth == 0)
+ return 0;
+
+ offset = offsets[0];
+ blk = EXT4_I(inode)->i_data[offset];
+ if (blk == 0) {
+ for (i = *offsets; i < EXT4_NDIR_BLOCKS; i++) {
+ if (EXT4_I(inode)->i_data[i] == 0)
+ hole_size++;
+ else
+ break;
+ }
+ return hole_size;
+ }
+
+
+ for (j = 1; j < depth; j++) {
+ offset = offsets[j];
+ bh = sb_getblk(sb, le32_to_cpu(blk));
+ if (unlikely(!bh))
+ return -EIO;
+
+ if (!bh_uptodate_or_lock(bh)) {
+ if (bh_submit_read(bh) < 0) {
+ put_bh(bh);
+ return -EIO;
+ }
+ /* validate block references */
+ if (ext4_check_indirect_blockref(inode, bh)) {
+ put_bh(bh);
+ return -EIO;
+ }
+ }
+
+ blk = ((__le32 *)bh->b_data)[offset];
+ /* Reader: end */
+ if (blk == 0) {
+ children = (__le32 *)(bh->b_data);
+ for (i = offset; i < offset +
+ blocks_to_boundary; i++) {
+ if (children[i] == 0)
+ hole_size++;
+ else
+ break;
+ }
+ return hole_size;
+ }
+ }
+
+ return 0;
+}
+
struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
ext4_lblk_t block, int create, int *err)
{
--
1.7.1
This patch adds two new routines: ext4_secure_delete_pblks
and ext4_secure_delete_lblks.
ext4_secure_delete_pblks() will write zeros to the specified
physical blocks or random data if the EXT4_SECRM_RANDOM_FL flag is
set. If the device supports secure discard, the secure
discard will be used instead. ext4_secure_delete_lblks handels walking
the logical blocks of a file and calling ext4_secure_delete_pblks()
as needed.
Signed-off-by: Allison Henderson <[email protected]>
---
v1->v2
Removed check for discard mount option and replaced with
check for secure discard and discard_zeroes_data
Added BLKDEV_DISCARD_SECURE to the sb_issue_discard call
v2->v3
Removed code for discard. A seperate patch will
be done to add that code in the block layer
v3->v4
Discard code will be kept in the vfs layer. Code
for secure delete is now in its own function,
ext4_secure_delete_pblks and is called
by a new function ext4_secure_delete_lblks
before any blocks are released
:100644 100644 5c9f88c... 34f82a1... M fs/ext4/ext4.h
:100644 100644 095c36f... 10180e3... M fs/ext4/ext4_extents.h
:100644 100644 57cf568... 40d4e50... M fs/ext4/extents.c
:100644 100644 9dc8c14... 0a526c4... M fs/ext4/inode.c
fs/ext4/ext4.h | 5 +
fs/ext4/ext4_extents.h | 2 +
fs/ext4/extents.c | 2 +-
fs/ext4/inode.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 204 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 5c9f88c..34f82a1 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2220,6 +2220,11 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
struct ext4_map_blocks *map, int flags);
extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
__u64 start, __u64 len);
+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);
+
/* move_extent.c */
extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
__u64 start_orig, __u64 start_donor,
diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index 095c36f..10180e3 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -290,5 +290,7 @@ extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
struct ext4_ext_path *);
extern void ext4_ext_drop_refs(struct ext4_ext_path *);
extern int ext4_ext_check_inode(struct inode *inode);
+extern int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
+ struct ext4_ext_cache *ex);
#endif /* _EXT4_EXTENTS */
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 57cf568..40d4e50 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2034,7 +2034,7 @@ ext4_ext_put_gap_in_cache(struct inode *inode, struct ext4_ext_path *path,
*
* Return 0 if cache is invalid; 1 if the cache is valid
*/
-static int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
+int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
struct ext4_ext_cache *ex){
struct ext4_ext_cache *cex;
struct ext4_sb_info *sbi;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9dc8c14..0a526c4 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -38,6 +38,7 @@
#include <linux/printk.h>
#include <linux/slab.h>
#include <linux/ratelimit.h>
+#include <linux/random.h>
#include "ext4_jbd2.h"
#include "xattr.h"
@@ -713,6 +714,201 @@ static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
return 0;
}
+
+/*
+ * ext4_secure_delete_pblks
+ *
+ * Securely delete physical blocks.
+ * If the devices supports secure discard,
+ * blocks will be discarded. Otherwise
+ * the blocks will be either zeroed or
+ * randomized if the random secure delete
+ * flag is on
+ *
+ * inode: The files inode
+ * block: The physical block at which to start deleteing
+ * count: The number of blocks to delete
+ *
+ * Returns 0 on sucess or negative on error
+ */
+int ext4_secure_delete_pblks(struct inode *inode, ext4_fsblk_t block,
+ unsigned long count){
+
+ struct fstrim_range range;
+ ext4_fsblk_t iblock, last_block;
+ struct buffer_head *bh;
+ struct super_block *sb = inode->i_sb;
+ struct request_queue *q = bdev_get_queue(sb->s_bdev);
+ struct ext4_super_block *es = EXT4_SB(inode->i_sb)->s_es;
+ int err = 0;
+
+ last_block = block + count;
+ /*
+ * Check to see if the device supports secure discard,
+ * And also that read after discard returns zeros
+ */
+ if (blk_queue_secdiscard(q) && q->limits.discard_zeroes_data) {
+ err = sb_issue_discard(sb, block, count,
+ GFP_NOFS, BLKDEV_DISCARD_SECURE);
+ if (err)
+ goto zero_out;
+
+ range.start = block;
+ range.len = count;
+ range.minlen = 1;
+ err = ext4_trim_fs(sb, &range);
+
+ if (err)
+ goto zero_out;
+
+ return 0;
+ }
+
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_RANDOM_FL) {
+ for (iblock = block; iblock < last_block; iblock++) {
+ bh = sb_getblk(sb, iblock);
+ get_random_bytes(bh->b_data, bh->b_size);
+ set_buffer_dirty(bh);
+
+ sync_dirty_buffer(bh);
+ if (buffer_req(bh) && !buffer_uptodate(bh)) {
+ es->s_last_error_block =
+ cpu_to_le64(bh->b_blocknr);
+ ext4_error_inode(inode, __func__,
+ __LINE__, bh->b_blocknr,
+ "IO error syncing itable block");
+ err = -EIO;
+ brelse(bh);
+ goto zero_out;
+ }
+ brelse(bh);
+ }
+ return 0;
+ }
+
+zero_out:
+ return sb_issue_zeroout(sb, block, count, GFP_NOFS);
+
+}
+
+/*
+ * ext4_secure_delete_lblks
+ *
+ * Secure deletes the data blocks of a file
+ * starting at the given logical block
+ *
+ * @inode: The files inode
+ * @first_block: Starting logical block
+ * @count: The number of blocks to secure delete
+ *
+ * Returns 0 on sucess or negative on error
+ */
+int ext4_secure_delete_lblks(struct inode *inode, ext4_lblk_t first_block,
+ unsigned long count){
+ handle_t *handle;
+ struct ext4_map_blocks map;
+ struct ext4_ext_cache cache_ex;
+ ext4_lblk_t num_blocks, max_blocks = 0;
+ ext4_lblk_t last_block = first_block + count;
+ ext4_lblk_t iblock = first_block;
+ int ret, credits, hole_len, err = 0;
+
+ credits = ext4_writepage_trans_blocks(inode);
+ handle = ext4_journal_start(inode, credits);
+ if (IS_ERR(handle))
+ return PTR_ERR(handle);
+
+ down_write(&EXT4_I(inode)->i_data_sem);
+ ext4_ext_invalidate_cache(inode);
+ ext4_discard_preallocations(inode);
+
+ /* Do not allow last_block to wrap when caller passes EXT_MAX_BLOCK */
+ if (last_block < first_block)
+ last_block = EXT_MAX_BLOCKS;
+
+ while (iblock < last_block) {
+ max_blocks = last_block - iblock;
+ num_blocks = 1;
+ memset(&map, 0, sizeof(map));
+ map.m_lblk = iblock;
+ map.m_len = max_blocks;
+
+ if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+ ret = ext4_ext_map_blocks(handle, inode, &map, 0);
+ else
+ ret = ext4_ind_map_blocks(handle, inode, &map, 0);
+
+ if (ret > 0) {
+ err = ext4_secure_delete_pblks(inode,
+ map.m_pblk, map.m_len);
+ if (err)
+ break;
+ num_blocks = ret;
+ } else if (ret == 0) {
+ if (ext4_test_inode_flag(inode,
+ EXT4_INODE_EXTENTS)) {
+ /*
+ * If map blocks could not find the block,
+ * then it is in a hole. If the hole was
+ * not already cached, then map blocks should
+ * put it in the cache. So we can get the hole
+ * out of the cache
+ */
+ memset(&cache_ex, 0, sizeof(cache_ex));
+ if ((ext4_ext_check_cache(inode, iblock,
+ &cache_ex)) && !cache_ex.ec_start) {
+
+ /* The hole is cached */
+ num_blocks = cache_ex.ec_block +
+ cache_ex.ec_len - iblock;
+
+ } else {
+ /* reached EOF of extent file */
+ break;
+ }
+ } else {
+ hole_len = ext4_ind_hole_lookup(inode, iblock);
+
+ if (hole_len > 0) {
+ /* Skip over the hole */
+ num_blocks = hole_len;
+ } else if (hole_len == 0) {
+ /* No hole, EOF reached */
+ break;
+ } else {
+ /* Hole look up err */
+ err = hole_len;
+ break;
+ }
+ }
+ } else {
+ /* Map blocks error */
+ err = ret;
+ break;
+ }
+
+ if (num_blocks == 0) {
+ /* This condition should never happen */
+ ext_debug("Block lookup failed");
+ err = -EIO;
+ break;
+ }
+
+ iblock += num_blocks;
+ }
+
+ if (IS_SYNC(inode))
+ ext4_handle_sync(handle);
+
+ up_write(&EXT4_I(inode)->i_data_sem);
+
+ inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
+ ext4_mark_inode_dirty(handle, inode);
+ ext4_journal_stop(handle);
+
+ return err;
+}
+
struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
ext4_lblk_t block, int create, int *err)
{
--
1.7.1
This patch modifies punch hole and truncate to securely
delete the data blocks of a file.
During a truncate or punch hole, files that have the EXT4_SECRM_FL
attribute flag on will have their blocks secure deleted before
they are released.
Signed-off-by: Allison Henderson <[email protected]>
---
:100644 100644 40d4e50... 984fac2... M fs/ext4/extents.c
:100644 100644 0a526c4... bd1facd... M fs/ext4/inode.c
fs/ext4/extents.c | 7 +++++++
fs/ext4/inode.c | 12 ++++++++++++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 40d4e50..984fac2 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4201,6 +4201,13 @@ int ext4_ext_punch_hole(struct file *file, loff_t offset, loff_t length)
/* finish any pending end_io work */
ext4_flush_completed_IO(inode);
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
+ err = ext4_secure_delete_lblks(inode, first_block,
+ last_block - first_block);
+ if (err)
+ return err;
+ }
+
credits = ext4_writepage_trans_blocks(inode);
handle = ext4_journal_start(inode, credits);
if (IS_ERR(handle))
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0a526c4..bd1facd 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3420,6 +3420,8 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
*/
void ext4_truncate(struct inode *inode)
{
+ int err = 0;
+ ext4_lblk_t last_block;
trace_ext4_truncate_enter(inode);
if (!ext4_can_truncate(inode))
@@ -3430,6 +3432,16 @@ void ext4_truncate(struct inode *inode)
if (inode->i_size == 0 && !test_opt(inode->i_sb, NO_AUTO_DA_ALLOC))
ext4_set_inode_state(inode, EXT4_STATE_DA_ALLOC_CLOSE);
+ last_block = (inode->i_size + EXT4_BLOCK_SIZE(inode->i_sb)-1)
+ >> EXT4_BLOCK_SIZE_BITS(inode->i_sb);
+
+ if (EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
+ err = ext4_secure_delete_lblks(inode,
+ last_block, EXT_MAX_BLOCKS);
+ if (err)
+ return;
+ }
+
if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
ext4_ext_truncate(inode);
else
--
1.7.1
This patch adds a new attribute flag EXT4_SECRM_RANDOM_FL.
During a secure delete, this flag will cause blocks to be
overwritten with random data instead of zeros.
Signed-off-by: Allison Henderson <[email protected]>
---
:100644 100644 e717dfd... db54ce4... M fs/ext4/ext4.h
fs/ext4/ext4.h | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e717dfd..db54ce4 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -350,17 +350,18 @@ struct flex_groups {
#define EXT4_EXTENTS_FL 0x00080000 /* Inode uses extents */
#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
#define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
+#define EXT4_SECRM_RANDOM_FL 0x10000000 /* Use random data instead of zeros */
#define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
-#define EXT4_FL_USER_VISIBLE 0x004BDFFF /* User visible flags */
-#define EXT4_FL_USER_MODIFIABLE 0x004B80FF /* User modifiable flags */
+#define EXT4_FL_USER_VISIBLE 0x104BDFFF /* User visible flags */
+#define EXT4_FL_USER_MODIFIABLE 0x104B80FF /* User modifiable flags */
/* Flags that should be inherited by new inodes from their parent. */
#define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
- EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
+ EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL | EXT4_SECRM_RANDOM_FL)
/* Flags that are appropriate for regular files (all but dir-specific ones). */
#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
@@ -407,6 +408,7 @@ enum {
EXT4_INODE_EXTENTS = 19, /* Inode uses extents */
EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
+ EXT4_INODE_SECRM_RANDOM = 28, /* Use random data instead of zeros */
EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
};
@@ -453,6 +455,7 @@ static inline void ext4_check_flag_values(void)
CHECK_FLAG_VALUE(EXTENTS);
CHECK_FLAG_VALUE(EA_INODE);
CHECK_FLAG_VALUE(EOFBLOCKS);
+ CHECK_FLAG_VALUE(SECRM_RANDOM);
CHECK_FLAG_VALUE(RESERVED);
}
--
1.7.1
On 2011-10-07, at 1:10 AM, Allison Henderson <[email protected]> wrote:
> Sorry for the delay in getting this next version out.
> I had some tasks to take care of, and now I'm picking up my
> secure delete work again. I'm still not quite done yet,
> but a lot has changed and I wanted to update people so that
> we have an idea of where its going. Currently the patch
> deals with data blocks, meta blocks, directory entries,
> journal blocks, and also provides an option for secure
> deleting with random data instead of just zeros.
> I'm also planning on adding some more patches to
> deal with inodes and also a mount option that turns
> on secure delete by default. Im still not quite done
> debugging, but Im just sending it out early to get
> some more eyes on it. Feed back appreciated! :)
>
> v3->v4
> Added a new file attribute flag EXT4_SECRM_RANDOM_FL
> This flag causes the secure delete operations to over write
> blocks with random data instead of zeros.
Since inode flags are in short supply, and I suspect users that want this want it for all files, this should probably be a superblock flag?
> New function ext4_secure_delete_lblks added to walk
> data blocks and secure delete them before any blocks
> are removed.
>
> Meta blocks are secure deleted before they are
> released
>
> New function added to identify holes in ind files.
> Used by ext4_secure_delete_lblks to skip over holes
> during secure delete.
>
> Added another list in the journal structure to track
> journal blocks so that they can be secure deleted later.
>
> Added new ext4_secure_delete_jblks that secure deletes
> journal blocks that were used to journal the specified
> logical blocks
>
> Allison Henderson (7):
> ext4: Secure Delete: Add new EXT4_SECRM_RANDOM_FL flag
> ext4: Secure Delete: Add ext4_ind_hole_lookup function
> ext4: Secure Delete: Add secure delete functions
> ext4: Secure Delete: Secure delete file data
> ext4: Secure Delete: Secure delete directory entry
> ext4: Secure Delete: Secure delete meta data blocks
> ext4/jbd2: Secure Delete: Secure delete journal blocks
>
> fs/ext4/ext4.h | 28 +++-
> fs/ext4/ext4_extents.h | 2 +
> fs/ext4/extents.c | 21 +++-
> fs/ext4/indirect.c | 2 +-
> fs/ext4/inode.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/ext4/mballoc.c | 8 +
> fs/ext4/namei.c | 64 +++++++-
> fs/jbd2/commit.c | 6 +
> fs/jbd2/journal.c | 112 ++++++++++++++
> include/linux/jbd2.h | 21 +++
> 10 files changed, 642 insertions(+), 13 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 07, 2011 at 12:10:59AM -0700, Allison Henderson wrote:
> This patch adds a new attribute flag EXT4_SECRM_RANDOM_FL.
> During a secure delete, this flag will cause blocks to be
> overwritten with random data instead of zeros.
>
> Signed-off-by: Allison Henderson <[email protected]>
> ---
> :100644 100644 e717dfd... db54ce4... M fs/ext4/ext4.h
> fs/ext4/ext4.h | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index e717dfd..db54ce4 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -350,17 +350,18 @@ struct flex_groups {
> #define EXT4_EXTENTS_FL 0x00080000 /* Inode uses extents */
> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> #define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
> +#define EXT4_SECRM_RANDOM_FL 0x10000000 /* Use random data instead of zeros */
> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>
> -#define EXT4_FL_USER_VISIBLE 0x004BDFFF /* User visible flags */
> -#define EXT4_FL_USER_MODIFIABLE 0x004B80FF /* User modifiable flags */
> +#define EXT4_FL_USER_VISIBLE 0x104BDFFF /* User visible flags */
> +#define EXT4_FL_USER_MODIFIABLE 0x104B80FF /* User modifiable flags */
Is there a reason why this #define is 0x104BDFFF instead of a bunch of flags
or'd together in a manner similar to the one below it?
--D
>
> /* Flags that should be inherited by new inodes from their parent. */
> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
> EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
> EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
> EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
> - EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
> + EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL | EXT4_SECRM_RANDOM_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> #define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
> @@ -407,6 +408,7 @@ enum {
> EXT4_INODE_EXTENTS = 19, /* Inode uses extents */
> EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
> EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
> + EXT4_INODE_SECRM_RANDOM = 28, /* Use random data instead of zeros */
> EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
> };
>
> @@ -453,6 +455,7 @@ static inline void ext4_check_flag_values(void)
> CHECK_FLAG_VALUE(EXTENTS);
> CHECK_FLAG_VALUE(EA_INODE);
> CHECK_FLAG_VALUE(EOFBLOCKS);
> + CHECK_FLAG_VALUE(SECRM_RANDOM);
> CHECK_FLAG_VALUE(RESERVED);
> }
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 10/07/2011 08:21 AM, Andreas Dilger wrote:
> On 2011-10-07, at 1:10 AM, Allison Henderson<[email protected]> wrote:
>> Sorry for the delay in getting this next version out.
>> I had some tasks to take care of, and now I'm picking up my
>> secure delete work again. I'm still not quite done yet,
>> but a lot has changed and I wanted to update people so that
>> we have an idea of where its going. Currently the patch
>> deals with data blocks, meta blocks, directory entries,
>> journal blocks, and also provides an option for secure
>> deleting with random data instead of just zeros.
>> I'm also planning on adding some more patches to
>> deal with inodes and also a mount option that turns
>> on secure delete by default. Im still not quite done
>> debugging, but Im just sending it out early to get
>> some more eyes on it. Feed back appreciated! :)
>>
>> v3->v4
>> Added a new file attribute flag EXT4_SECRM_RANDOM_FL
>> This flag causes the secure delete operations to over write
>> blocks with random data instead of zeros.
>
> Since inode flags are in short supply, and I suspect users that want this want it for all files, this should probably be a superblock flag?
>
That is a really good point. The first thing that comes to mind though would be the fact that it is a lot slower when the random flag is on especially for really big files. So that would be one case where I could imagine a user might want the ability to set different options per file. But since the flags are a limited resource, I can see where we may not want to spend it so quickly. I will see if maybe there is some way I can optimize it, but I would like to see more folks weigh in on this topic too.
>> New function ext4_secure_delete_lblks added to walk
>> data blocks and secure delete them before any blocks
>> are removed.
>>
>> Meta blocks are secure deleted before they are
>> released
>>
>> New function added to identify holes in ind files.
>> Used by ext4_secure_delete_lblks to skip over holes
>> during secure delete.
>>
>> Added another list in the journal structure to track
>> journal blocks so that they can be secure deleted later.
>>
>> Added new ext4_secure_delete_jblks that secure deletes
>> journal blocks that were used to journal the specified
>> logical blocks
>>
>> Allison Henderson (7):
>> ext4: Secure Delete: Add new EXT4_SECRM_RANDOM_FL flag
>> ext4: Secure Delete: Add ext4_ind_hole_lookup function
>> ext4: Secure Delete: Add secure delete functions
>> ext4: Secure Delete: Secure delete file data
>> ext4: Secure Delete: Secure delete directory entry
>> ext4: Secure Delete: Secure delete meta data blocks
>> ext4/jbd2: Secure Delete: Secure delete journal blocks
>>
>> fs/ext4/ext4.h | 28 +++-
>> fs/ext4/ext4_extents.h | 2 +
>> fs/ext4/extents.c | 21 +++-
>> fs/ext4/indirect.c | 2 +-
>> fs/ext4/inode.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++
>> fs/ext4/mballoc.c | 8 +
>> fs/ext4/namei.c | 64 +++++++-
>> fs/jbd2/commit.c | 6 +
>> fs/jbd2/journal.c | 112 ++++++++++++++
>> include/linux/jbd2.h | 21 +++
>> 10 files changed, 642 insertions(+), 13 deletions(-)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 10/07/2011 10:02 AM, Darrick J. Wong wrote:
> On Fri, Oct 07, 2011 at 12:10:59AM -0700, Allison Henderson wrote:
>> This patch adds a new attribute flag EXT4_SECRM_RANDOM_FL.
>> During a secure delete, this flag will cause blocks to be
>> overwritten with random data instead of zeros.
>>
>> Signed-off-by: Allison Henderson<[email protected]>
>> ---
>> :100644 100644 e717dfd... db54ce4... M fs/ext4/ext4.h
>> fs/ext4/ext4.h | 9 ++++++---
>> 1 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index e717dfd..db54ce4 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -350,17 +350,18 @@ struct flex_groups {
>> #define EXT4_EXTENTS_FL 0x00080000 /* Inode uses extents */
>> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
>> #define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
>> +#define EXT4_SECRM_RANDOM_FL 0x10000000 /* Use random data instead of zeros */
>> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>>
>> -#define EXT4_FL_USER_VISIBLE 0x004BDFFF /* User visible flags */
>> -#define EXT4_FL_USER_MODIFIABLE 0x004B80FF /* User modifiable flags */
>> +#define EXT4_FL_USER_VISIBLE 0x104BDFFF /* User visible flags */
>> +#define EXT4_FL_USER_MODIFIABLE 0x104B80FF /* User modifiable flags */
>
> Is there a reason why this #define is 0x104BDFFF instead of a bunch of flags
> or'd together in a manner similar to the one below it?
>
> --D
That's a really good suggestion, and I dont see any reason why it needs
to be a hard number like that. I will definitely add that in if we
decide to keep the EXT4_SECRM_RANDOM_FL flag. Thx!
>>
>> /* Flags that should be inherited by new inodes from their parent. */
>> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
>> EXT4_SYNC_FL | EXT4_IMMUTABLE_FL | EXT4_APPEND_FL |\
>> EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
>> EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
>> - EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
>> + EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL | EXT4_SECRM_RANDOM_FL)
>>
>> /* Flags that are appropriate for regular files (all but dir-specific ones). */
>> #define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
>> @@ -407,6 +408,7 @@ enum {
>> EXT4_INODE_EXTENTS = 19, /* Inode uses extents */
>> EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
>> EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
>> + EXT4_INODE_SECRM_RANDOM = 28, /* Use random data instead of zeros */
>> EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
>> };
>>
>> @@ -453,6 +455,7 @@ static inline void ext4_check_flag_values(void)
>> CHECK_FLAG_VALUE(EXTENTS);
>> CHECK_FLAG_VALUE(EA_INODE);
>> CHECK_FLAG_VALUE(EOFBLOCKS);
>> + CHECK_FLAG_VALUE(SECRM_RANDOM);
>> CHECK_FLAG_VALUE(RESERVED);
>> }
>>
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
On 10/07/2011 12:11 AM, Allison Henderson wrote:
> This patch adds two new routines: ext4_secure_delete_pblks
> and ext4_secure_delete_lblks.
>
> ext4_secure_delete_pblks() will write zeros to the specified
> physical blocks or random data if the EXT4_SECRM_RANDOM_FL flag is
> set. If the device supports secure discard, the secure
> discard will be used instead. ext4_secure_delete_lblks handels walking
> the logical blocks of a file and calling ext4_secure_delete_pblks()
> as needed.
>
> Signed-off-by: Allison Henderson<[email protected]>
> ---
> v1->v2
> Removed check for discard mount option and replaced with
> check for secure discard and discard_zeroes_data
>
> Added BLKDEV_DISCARD_SECURE to the sb_issue_discard call
>
> v2->v3
> Removed code for discard. A seperate patch will
> be done to add that code in the block layer
>
> v3->v4
> Discard code will be kept in the vfs layer. Code
> for secure delete is now in its own function,
> ext4_secure_delete_pblks and is called
> by a new function ext4_secure_delete_lblks
> before any blocks are released
Typo here: we decided to keep the discard code in the fs layer, not the
vfs layer. Sorry for the confusion :)
>
> :100644 100644 5c9f88c... 34f82a1... M fs/ext4/ext4.h
> :100644 100644 095c36f... 10180e3... M fs/ext4/ext4_extents.h
> :100644 100644 57cf568... 40d4e50... M fs/ext4/extents.c
> :100644 100644 9dc8c14... 0a526c4... M fs/ext4/inode.c
> fs/ext4/ext4.h | 5 +
> fs/ext4/ext4_extents.h | 2 +
> fs/ext4/extents.c | 2 +-
> fs/ext4/inode.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 204 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 5c9f88c..34f82a1 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2220,6 +2220,11 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
> struct ext4_map_blocks *map, int flags);
> extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> __u64 start, __u64 len);
> +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);
> +
> /* move_extent.c */
> extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
> __u64 start_orig, __u64 start_donor,
> diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> index 095c36f..10180e3 100644
> --- a/fs/ext4/ext4_extents.h
> +++ b/fs/ext4/ext4_extents.h
> @@ -290,5 +290,7 @@ extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
> struct ext4_ext_path *);
> extern void ext4_ext_drop_refs(struct ext4_ext_path *);
> extern int ext4_ext_check_inode(struct inode *inode);
> +extern int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> + struct ext4_ext_cache *ex);
> #endif /* _EXT4_EXTENTS */
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 57cf568..40d4e50 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2034,7 +2034,7 @@ ext4_ext_put_gap_in_cache(struct inode *inode, struct ext4_ext_path *path,
> *
> * Return 0 if cache is invalid; 1 if the cache is valid
> */
> -static int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> +int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> struct ext4_ext_cache *ex){
> struct ext4_ext_cache *cex;
> struct ext4_sb_info *sbi;
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 9dc8c14..0a526c4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -38,6 +38,7 @@
> #include<linux/printk.h>
> #include<linux/slab.h>
> #include<linux/ratelimit.h>
> +#include<linux/random.h>
>
> #include "ext4_jbd2.h"
> #include "xattr.h"
> @@ -713,6 +714,201 @@ static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
> return 0;
> }
>
> +
> +/*
> + * ext4_secure_delete_pblks
> + *
> + * Securely delete physical blocks.
> + * If the devices supports secure discard,
> + * blocks will be discarded. Otherwise
> + * the blocks will be either zeroed or
> + * randomized if the random secure delete
> + * flag is on
> + *
> + * inode: The files inode
> + * block: The physical block at which to start deleteing
> + * count: The number of blocks to delete
> + *
> + * Returns 0 on sucess or negative on error
> + */
> +int ext4_secure_delete_pblks(struct inode *inode, ext4_fsblk_t block,
> + unsigned long count){
> +
> + struct fstrim_range range;
> + ext4_fsblk_t iblock, last_block;
> + struct buffer_head *bh;
> + struct super_block *sb = inode->i_sb;
> + struct request_queue *q = bdev_get_queue(sb->s_bdev);
> + struct ext4_super_block *es = EXT4_SB(inode->i_sb)->s_es;
> + int err = 0;
> +
> + last_block = block + count;
> + /*
> + * Check to see if the device supports secure discard,
> + * And also that read after discard returns zeros
> + */
> + if (blk_queue_secdiscard(q)&& q->limits.discard_zeroes_data) {
> + err = sb_issue_discard(sb, block, count,
> + GFP_NOFS, BLKDEV_DISCARD_SECURE);
> + if (err)
> + goto zero_out;
> +
> + range.start = block;
> + range.len = count;
> + range.minlen = 1;
> + err = ext4_trim_fs(sb,&range);
> +
> + if (err)
> + goto zero_out;
> +
> + return 0;
> + }
> +
> + if (EXT4_I(inode)->i_flags& EXT4_SECRM_RANDOM_FL) {
> + for (iblock = block; iblock< last_block; iblock++) {
> + bh = sb_getblk(sb, iblock);
> + get_random_bytes(bh->b_data, bh->b_size);
> + set_buffer_dirty(bh);
> +
> + sync_dirty_buffer(bh);
> + if (buffer_req(bh)&& !buffer_uptodate(bh)) {
> + es->s_last_error_block =
> + cpu_to_le64(bh->b_blocknr);
> + ext4_error_inode(inode, __func__,
> + __LINE__, bh->b_blocknr,
> + "IO error syncing itable block");
> + err = -EIO;
> + brelse(bh);
> + goto zero_out;
> + }
> + brelse(bh);
> + }
> + return 0;
> + }
> +
> +zero_out:
> + return sb_issue_zeroout(sb, block, count, GFP_NOFS);
> +
> +}
> +
> +/*
> + * ext4_secure_delete_lblks
> + *
> + * Secure deletes the data blocks of a file
> + * starting at the given logical block
> + *
> + * @inode: The files inode
> + * @first_block: Starting logical block
> + * @count: The number of blocks to secure delete
> + *
> + * Returns 0 on sucess or negative on error
> + */
> +int ext4_secure_delete_lblks(struct inode *inode, ext4_lblk_t first_block,
> + unsigned long count){
> + handle_t *handle;
> + struct ext4_map_blocks map;
> + struct ext4_ext_cache cache_ex;
> + ext4_lblk_t num_blocks, max_blocks = 0;
> + ext4_lblk_t last_block = first_block + count;
> + ext4_lblk_t iblock = first_block;
> + int ret, credits, hole_len, err = 0;
> +
> + credits = ext4_writepage_trans_blocks(inode);
> + handle = ext4_journal_start(inode, credits);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + down_write(&EXT4_I(inode)->i_data_sem);
> + ext4_ext_invalidate_cache(inode);
> + ext4_discard_preallocations(inode);
> +
> + /* Do not allow last_block to wrap when caller passes EXT_MAX_BLOCK */
> + if (last_block< first_block)
> + last_block = EXT_MAX_BLOCKS;
> +
> + while (iblock< last_block) {
> + max_blocks = last_block - iblock;
> + num_blocks = 1;
> + memset(&map, 0, sizeof(map));
> + map.m_lblk = iblock;
> + map.m_len = max_blocks;
> +
> + if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> + ret = ext4_ext_map_blocks(handle, inode,&map, 0);
> + else
> + ret = ext4_ind_map_blocks(handle, inode,&map, 0);
> +
> + if (ret> 0) {
> + err = ext4_secure_delete_pblks(inode,
> + map.m_pblk, map.m_len);
> + if (err)
> + break;
> + num_blocks = ret;
> + } else if (ret == 0) {
> + if (ext4_test_inode_flag(inode,
> + EXT4_INODE_EXTENTS)) {
> + /*
> + * If map blocks could not find the block,
> + * then it is in a hole. If the hole was
> + * not already cached, then map blocks should
> + * put it in the cache. So we can get the hole
> + * out of the cache
> + */
> + memset(&cache_ex, 0, sizeof(cache_ex));
> + if ((ext4_ext_check_cache(inode, iblock,
> + &cache_ex))&& !cache_ex.ec_start) {
> +
> + /* The hole is cached */
> + num_blocks = cache_ex.ec_block +
> + cache_ex.ec_len - iblock;
> +
> + } else {
> + /* reached EOF of extent file */
> + break;
> + }
> + } else {
> + hole_len = ext4_ind_hole_lookup(inode, iblock);
> +
> + if (hole_len> 0) {
> + /* Skip over the hole */
> + num_blocks = hole_len;
> + } else if (hole_len == 0) {
> + /* No hole, EOF reached */
> + break;
> + } else {
> + /* Hole look up err */
> + err = hole_len;
> + break;
> + }
> + }
> + } else {
> + /* Map blocks error */
> + err = ret;
> + break;
> + }
> +
> + if (num_blocks == 0) {
> + /* This condition should never happen */
> + ext_debug("Block lookup failed");
> + err = -EIO;
> + break;
> + }
> +
> + iblock += num_blocks;
> + }
> +
> + if (IS_SYNC(inode))
> + ext4_handle_sync(handle);
> +
> + up_write(&EXT4_I(inode)->i_data_sem);
> +
> + inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
> + ext4_mark_inode_dirty(handle, inode);
> + ext4_journal_stop(handle);
> +
> + return err;
> +}
> +
> struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
> ext4_lblk_t block, int create, int *err)
> {
On Fri, Oct 07, 2011 at 12:11:03AM -0700, Allison Henderson wrote:
> This patch zeros or randomizes a files directory entry when a file
<snip>
> @@ -1669,7 +1672,38 @@ static int ext4_delete_entry(handle_t *handle,
> de->inode = 0;
> dir->i_version++;
> BUFFER_TRACE(bh, "call ext4_handle_dirty_metadata");
> - err = ext4_handle_dirty_metadata(handle, dir, bh);
> +
> + /*
> + * If the secure remove flag is on, zero
> + * or randomize the entry and write it out
> + * to the disk
> + */
> + if (flags & EXT4_DEL_ENTRY_ZERO) {
> + memset(de->name, 0x00, de->name_len);
> + de->file_type = 0;
> + } else if (flags & EXT4_DEL_ENTRY_RAND) {
> + get_random_bytes(de->name, de->name_len);
> + get_random_bytes(&(de->file_type),
> + sizeof(de->file_type));
Seeing as name_len and file_type precede name, you could probably reduce this
to one call that zaps all three. Same for the zero-out case.
Also you might want to make both of these name-erasing calls zap the space
between the end of the name and the end of the record, just in case there's old
filename component still lurking in that space. (Yes, paranoia...)
> + }
> +
> + if (flags & EXT4_DEL_ENTRY_ZERO ||
> + flags & EXT4_DEL_ENTRY_RAND) {
> +
> + set_buffer_dirty(bh);
> + sync_dirty_buffer(bh);
> + if (buffer_req(bh) && !buffer_uptodate(bh)) {
> + es->s_last_error_block =
> + cpu_to_le64(bh->b_blocknr);
> + ext4_error_inode(dir, __func__,
> + __LINE__, bh->b_blocknr,
> + "IO error syncing itable block");
itable?
--D
On Fri, Oct 07, 2011 at 12:11:00AM -0700, Allison Henderson wrote:
> This patch adds a new ext4_ind_hole_lookup function that
> will be used to identify and skip over holes during a
> secure delete operation
>
> Signed-off-by: Allison Henderson <[email protected]>
> ---
> :100644 100644 db54ce4... 5c9f88c... M fs/ext4/ext4.h
> :100644 100644 0962642... 2fe9dfd... M fs/ext4/indirect.c
> :100644 100644 c4da98a... 9dc8c14... M fs/ext4/inode.c
> fs/ext4/ext4.h | 5 +++
> fs/ext4/indirect.c | 2 +-
> fs/ext4/inode.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 79 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index db54ce4..5c9f88c 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2241,6 +2241,11 @@ extern int ext4_bio_write_page(struct ext4_io_submit *io,
> /* mmp.c */
> extern int ext4_multi_mount_protect(struct super_block *, ext4_fsblk_t);
>
> +/* indirects.c */
> +extern int ext4_block_to_path(struct inode *inode,
> + ext4_lblk_t i_block,
> + ext4_lblk_t offsets[4], int *boundary);
> +
> /* BH_Uninit flag: blocks are allocated but uninitialized on disk */
> enum ext4_state_bits {
> BH_Uninit /* blocks are allocated but uninitialized on disk */
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index 0962642..2fe9dfd 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -69,7 +69,7 @@ static inline void add_chain(Indirect *p, struct buffer_head *bh, __le32 *v)
> * get there at all.
> */
>
> -static int ext4_block_to_path(struct inode *inode,
> +int ext4_block_to_path(struct inode *inode,
> ext4_lblk_t i_block,
> ext4_lblk_t offsets[4], int *boundary)
> {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c4da98a..9dc8c14 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -640,6 +640,79 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
> return bh;
> }
>
> +/*
> + * ext4_in_hole_lookup
> + *
> + * Returns the size of the hole located at the given block up to
> + * the first boundry, or 0 if there is no hole. Returns negative on err.
boundary
> + */
> +static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
> +{
> + struct super_block *sb = inode->i_sb;
> + struct buffer_head *bh;
> + ext4_lblk_t offsets[4];
> + ext4_lblk_t offset;
> + __le32 *children;
> + __le32 blk;
> + int blocks_to_boundary = 0;
> + int depth = 0;
> + int hole_size = 0;
> + unsigned int i, j;
> +
> + depth = ext4_block_to_path(inode, block, offsets, &blocks_to_boundary);
> +
> + if (depth == 0)
> + return 0;
> +
> + offset = offsets[0];
> + blk = EXT4_I(inode)->i_data[offset];
> + if (blk == 0) {
> + for (i = *offsets; i < EXT4_NDIR_BLOCKS; i++) {
> + if (EXT4_I(inode)->i_data[i] == 0)
> + hole_size++;
> + else
> + break;
> + }
> + return hole_size;
> + }
> +
> +
> + for (j = 1; j < depth; j++) {
> + offset = offsets[j];
> + bh = sb_getblk(sb, le32_to_cpu(blk));
> + if (unlikely(!bh))
> + return -EIO;
> +
> + if (!bh_uptodate_or_lock(bh)) {
> + if (bh_submit_read(bh) < 0) {
> + put_bh(bh);
> + return -EIO;
> + }
> + /* validate block references */
> + if (ext4_check_indirect_blockref(inode, bh)) {
> + put_bh(bh);
> + return -EIO;
> + }
> + }
> +
> + blk = ((__le32 *)bh->b_data)[offset];
> + /* Reader: end */
> + if (blk == 0) {
> + children = (__le32 *)(bh->b_data);
> + for (i = offset; i < offset +
> + blocks_to_boundary; i++) {
> + if (children[i] == 0)
> + hole_size++;
> + else
> + break;
> + }
Do you need a put_bh here somewhere?
--D
On 10/07/2011 10:22 AM, Darrick J. Wong wrote:
> On Fri, Oct 07, 2011 at 12:11:03AM -0700, Allison Henderson wrote:
>> This patch zeros or randomizes a files directory entry when a file
> <snip>
>> @@ -1669,7 +1672,38 @@ static int ext4_delete_entry(handle_t *handle,
>> de->inode = 0;
>> dir->i_version++;
>> BUFFER_TRACE(bh, "call ext4_handle_dirty_metadata");
>> - err = ext4_handle_dirty_metadata(handle, dir, bh);
>> +
>> + /*
>> + * If the secure remove flag is on, zero
>> + * or randomize the entry and write it out
>> + * to the disk
>> + */
>> + if (flags& EXT4_DEL_ENTRY_ZERO) {
>> + memset(de->name, 0x00, de->name_len);
>> + de->file_type = 0;
>> + } else if (flags& EXT4_DEL_ENTRY_RAND) {
>> + get_random_bytes(de->name, de->name_len);
>> + get_random_bytes(&(de->file_type),
>> + sizeof(de->file_type));
>
> Seeing as name_len and file_type precede name, you could probably reduce this
> to one call that zaps all three. Same for the zero-out case.
>
> Also you might want to make both of these name-erasing calls zap the space
> between the end of the name and the end of the record, just in case there's old
> filename component still lurking in that space. (Yes, paranoia...)
Alrighty, sounds good to me.
>
>> + }
>> +
>> + if (flags& EXT4_DEL_ENTRY_ZERO ||
>> + flags& EXT4_DEL_ENTRY_RAND) {
>> +
>> + set_buffer_dirty(bh);
>> + sync_dirty_buffer(bh);
>> + if (buffer_req(bh)&& !buffer_uptodate(bh)) {
>> + es->s_last_error_block =
>> + cpu_to_le64(bh->b_blocknr);
>> + ext4_error_inode(dir, __func__,
>> + __LINE__, bh->b_blocknr,
>> + "IO error syncing itable block");
>
> itable?
Sorry, I borrowed this snippet from another area of code and forgot to update the error message. Will fix :)
>
> --D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Fri, Oct 07, 2011 at 12:11:01AM -0700, Allison Henderson wrote:
> This patch adds two new routines: ext4_secure_delete_pblks
> and ext4_secure_delete_lblks.
>
> ext4_secure_delete_pblks() will write zeros to the specified
> physical blocks or random data if the EXT4_SECRM_RANDOM_FL flag is
> set. If the device supports secure discard, the secure
> discard will be used instead. ext4_secure_delete_lblks handels walking
handles
> the logical blocks of a file and calling ext4_secure_delete_pblks()
> as needed.
>
> Signed-off-by: Allison Henderson <[email protected]>
> ---
> v1->v2
> Removed check for discard mount option and replaced with
> check for secure discard and discard_zeroes_data
>
> Added BLKDEV_DISCARD_SECURE to the sb_issue_discard call
>
> v2->v3
> Removed code for discard. A seperate patch will
separate
> be done to add that code in the block layer
>
> v3->v4
> Discard code will be kept in the vfs layer. Code
> for secure delete is now in its own function,
> ext4_secure_delete_pblks and is called
> by a new function ext4_secure_delete_lblks
> before any blocks are released
>
> :100644 100644 5c9f88c... 34f82a1... M fs/ext4/ext4.h
> :100644 100644 095c36f... 10180e3... M fs/ext4/ext4_extents.h
> :100644 100644 57cf568... 40d4e50... M fs/ext4/extents.c
> :100644 100644 9dc8c14... 0a526c4... M fs/ext4/inode.c
> fs/ext4/ext4.h | 5 +
> fs/ext4/ext4_extents.h | 2 +
> fs/ext4/extents.c | 2 +-
> fs/ext4/inode.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 204 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 5c9f88c..34f82a1 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2220,6 +2220,11 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
> struct ext4_map_blocks *map, int flags);
> extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> __u64 start, __u64 len);
> +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);
> +
> /* move_extent.c */
> extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
> __u64 start_orig, __u64 start_donor,
> diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> index 095c36f..10180e3 100644
> --- a/fs/ext4/ext4_extents.h
> +++ b/fs/ext4/ext4_extents.h
> @@ -290,5 +290,7 @@ extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
> struct ext4_ext_path *);
> extern void ext4_ext_drop_refs(struct ext4_ext_path *);
> extern int ext4_ext_check_inode(struct inode *inode);
> +extern int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> + struct ext4_ext_cache *ex);
> #endif /* _EXT4_EXTENTS */
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 57cf568..40d4e50 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2034,7 +2034,7 @@ ext4_ext_put_gap_in_cache(struct inode *inode, struct ext4_ext_path *path,
> *
> * Return 0 if cache is invalid; 1 if the cache is valid
> */
> -static int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> +int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
> struct ext4_ext_cache *ex){
> struct ext4_ext_cache *cex;
> struct ext4_sb_info *sbi;
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 9dc8c14..0a526c4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -38,6 +38,7 @@
> #include <linux/printk.h>
> #include <linux/slab.h>
> #include <linux/ratelimit.h>
> +#include <linux/random.h>
>
> #include "ext4_jbd2.h"
> #include "xattr.h"
> @@ -713,6 +714,201 @@ static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
> return 0;
> }
>
> +
> +/*
> + * ext4_secure_delete_pblks
> + *
> + * Securely delete physical blocks.
> + * If the devices supports secure discard,
> + * blocks will be discarded. Otherwise
> + * the blocks will be either zeroed or
> + * randomized if the random secure delete
> + * flag is on
The fact that random secure delete produces zeroed blocks on discard devices is
documented somewhere user-visible, right? Just in case someone actually
depends on the randomizing.
> + * inode: The files inode
> + * block: The physical block at which to start deleteing
deleting
> + * count: The number of blocks to delete
> + *
> + * Returns 0 on sucess or negative on error
> + */
> +int ext4_secure_delete_pblks(struct inode *inode, ext4_fsblk_t block,
> + unsigned long count){
> +
> + struct fstrim_range range;
> + ext4_fsblk_t iblock, last_block;
> + struct buffer_head *bh;
> + struct super_block *sb = inode->i_sb;
> + struct request_queue *q = bdev_get_queue(sb->s_bdev);
> + struct ext4_super_block *es = EXT4_SB(inode->i_sb)->s_es;
> + int err = 0;
> +
> + last_block = block + count;
> + /*
> + * Check to see if the device supports secure discard,
> + * And also that read after discard returns zeros
> + */
> + if (blk_queue_secdiscard(q) && q->limits.discard_zeroes_data) {
> + err = sb_issue_discard(sb, block, count,
> + GFP_NOFS, BLKDEV_DISCARD_SECURE);
> + if (err)
> + goto zero_out;
> +
> + range.start = block;
> + range.len = count;
> + range.minlen = 1;
> + err = ext4_trim_fs(sb, &range);
> +
> + if (err)
> + goto zero_out;
> +
> + return 0;
> + }
> +
> + if (EXT4_I(inode)->i_flags & EXT4_SECRM_RANDOM_FL) {
> + for (iblock = block; iblock < last_block; iblock++) {
> + bh = sb_getblk(sb, iblock);
> + get_random_bytes(bh->b_data, bh->b_size);
> + set_buffer_dirty(bh);
> +
> + sync_dirty_buffer(bh);
> + if (buffer_req(bh) && !buffer_uptodate(bh)) {
> + es->s_last_error_block =
> + cpu_to_le64(bh->b_blocknr);
> + ext4_error_inode(inode, __func__,
> + __LINE__, bh->b_blocknr,
> + "IO error syncing itable block");
itable block?
> + err = -EIO;
> + brelse(bh);
> + goto zero_out;
> + }
> + brelse(bh);
> + }
> + return 0;
> + }
> +
> +zero_out:
> + return sb_issue_zeroout(sb, block, count, GFP_NOFS);
> +
> +}
> +
> +/*
> + * ext4_secure_delete_lblks
> + *
> + * Secure deletes the data blocks of a file
> + * starting at the given logical block
> + *
> + * @inode: The files inode
> + * @first_block: Starting logical block
> + * @count: The number of blocks to secure delete
> + *
> + * Returns 0 on sucess or negative on error
> + */
> +int ext4_secure_delete_lblks(struct inode *inode, ext4_lblk_t first_block,
> + unsigned long count){
> + handle_t *handle;
> + struct ext4_map_blocks map;
> + struct ext4_ext_cache cache_ex;
> + ext4_lblk_t num_blocks, max_blocks = 0;
> + ext4_lblk_t last_block = first_block + count;
> + ext4_lblk_t iblock = first_block;
> + int ret, credits, hole_len, err = 0;
> +
> + credits = ext4_writepage_trans_blocks(inode);
> + handle = ext4_journal_start(inode, credits);
> + if (IS_ERR(handle))
> + return PTR_ERR(handle);
> +
> + down_write(&EXT4_I(inode)->i_data_sem);
> + ext4_ext_invalidate_cache(inode);
> + ext4_discard_preallocations(inode);
> +
> + /* Do not allow last_block to wrap when caller passes EXT_MAX_BLOCK */
> + if (last_block < first_block)
> + last_block = EXT_MAX_BLOCKS;
> +
> + while (iblock < last_block) {
> + max_blocks = last_block - iblock;
> + num_blocks = 1;
> + memset(&map, 0, sizeof(map));
> + map.m_lblk = iblock;
> + map.m_len = max_blocks;
> +
> + if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> + ret = ext4_ext_map_blocks(handle, inode, &map, 0);
> + else
> + ret = ext4_ind_map_blocks(handle, inode, &map, 0);
> +
> + if (ret > 0) {
> + err = ext4_secure_delete_pblks(inode,
> + map.m_pblk, map.m_len);
> + if (err)
> + break;
> + num_blocks = ret;
> + } else if (ret == 0) {
> + if (ext4_test_inode_flag(inode,
> + EXT4_INODE_EXTENTS)) {
> + /*
> + * If map blocks could not find the block,
> + * then it is in a hole. If the hole was
> + * not already cached, then map blocks should
> + * put it in the cache. So we can get the hole
> + * out of the cache
> + */
> + memset(&cache_ex, 0, sizeof(cache_ex));
> + if ((ext4_ext_check_cache(inode, iblock,
> + &cache_ex)) && !cache_ex.ec_start) {
> +
> + /* The hole is cached */
> + num_blocks = cache_ex.ec_block +
> + cache_ex.ec_len - iblock;
> +
> + } else {
> + /* reached EOF of extent file */
> + break;
> + }
> + } else {
> + hole_len = ext4_ind_hole_lookup(inode, iblock);
> +
> + if (hole_len > 0) {
> + /* Skip over the hole */
> + num_blocks = hole_len;
> + } else if (hole_len == 0) {
> + /* No hole, EOF reached */
> + break;
> + } else {
> + /* Hole look up err */
> + err = hole_len;
> + break;
> + }
> + }
> + } else {
> + /* Map blocks error */
> + err = ret;
> + break;
> + }
> +
> + if (num_blocks == 0) {
> + /* This condition should never happen */
> + ext_debug("Block lookup failed");
> + err = -EIO;
> + break;
> + }
> +
> + iblock += num_blocks;
> + }
> +
> + if (IS_SYNC(inode))
> + ext4_handle_sync(handle);
> +
> + up_write(&EXT4_I(inode)->i_data_sem);
> +
> + inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
> + ext4_mark_inode_dirty(handle, inode);
> + ext4_journal_stop(handle);
> +
> + return err;
> +}
> +
> struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
> ext4_lblk_t block, int create, int *err)
> {
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 <[email protected]>
> ---
> :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.
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
> + if ((b_pair->vfs_inode == inode) &&
> + (b_pair->vfs_lblk >= first_block) &&
> + (b_pair->vfs_lblk < last_block)) {
> +
> + /*
> + * It is likely that the journal blocks will be
> + * consecutive, so lets try to secure delete them
> + * in ranges
> + */
> + if (jbd2_pblk_count == 0) {
> + /*
> + * If there are no blocks in our range,
> + * then this one will be the first
> + */
> + jbd2_pblk_start = b_pair->jbd2_pblk;
> + jbd2_pblk_count = 1;
> + } else if ((b_pair->jbd2_pblk) ==
> + (jbd2_pblk_start + jbd2_pblk_count)) {
> + /*
> + * If this journal block is physically
> + * consecutive, then just increase the range
> + */
> + jbd2_pblk_count++;
> + } else if ((b_pair->jbd2_pblk) ==
> + (jbd2_pblk_start - 1) &&
> + jbd2_pblk_start > 0) {
> + /*
> + * If this journal block is physically
> + * consecutive (from the start of the
> + * range), just increase the range from the
> + * other end.
> + */
> + jbd2_pblk_count++;
> + jbd2_pblk_start--;
> + } else {
> + /*
> + * If the block was not consecutive, secure
> + * delete the range, and restart the current
> + * range
> + */
> +
> + err = ext4_secure_delete_pblks(journal->j_inode,
> + jbd2_pblk_start, jbd2_pblk_count);
> + if (err)
> + goto out;
> + jbd2_pblk_start = b_pair->jbd2_pblk;
> + jbd2_pblk_count = 1;
> + }
> + }
> + }
> +
> + /* Secure delete any blocks still in our range */
> + if (jbd2_pblk_count > 0)
> + err = ext4_secure_delete_pblks(journal->j_inode,
> + jbd2_pblk_start, jbd2_pblk_count);
> +
> +out:
> + spin_unlock(&journal->j_pair_lock);
> + return err;
> +}
> +
> struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
> ext4_lblk_t block, int create, int *err)
> {
> @@ -3447,6 +3551,12 @@ void ext4_truncate(struct inode *inode)
> else
> ext4_ind_truncate(inode);
>
> + if (EXT4_I(inode)->i_flags & EXT4_SECRM_FL) {
> + err = ext4_secure_delete_jblks(inode,
> + inode->i_size >> EXT4_BLOCK_SIZE_BITS(inode->i_sb),
> + EXT_MAX_BLOCKS);
> + }
> +
> trace_ext4_truncate_exit(inode);
> }
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index eef6979..2734e7b 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -593,6 +593,12 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> jbd2_journal_abort(journal, flags);
> continue;
> }
> +
> + err = jbd2_record_pair(journal, jh2bh(jh), jh2bh(new_jh));
> + if (err) {
> + jbd2_journal_abort(journal, flags);
> + continue;
> + }
> set_bit(BH_JWrite, &jh2bh(new_jh)->b_state);
> wbuf[bufs++] = jh2bh(new_jh);
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index f24df13..6dd8af7 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -248,6 +248,100 @@ static void journal_kill_thread(journal_t *journal)
> }
> write_unlock(&journal->j_state_lock);
> }
> +/*
> + * jbd2_record_pair()
> + * Updates the journals list of block pairs to contain an new pair of journal
> + * to vfs blocks. This list is an in memory record of the journals blocks to
> + * help facilitate finding out what vfs blocks the journal blocks contain.
> + * Secure delete uses this list to clear out stale journal blocks that may
> + * still contain file data that needs to be secure deleted. This list is updated
> + * when a vfs data block is written to the journal or when a descriptor block
> + * is written to the journal.
> + *
> + * journal: The journal to update
> + * vfs_bh: The vfs buffer_head that the jbd2_bh is journaling. This
> + * may be NULL if jbd2_bh does not represent a vfs block.
> + * jbd2_bh: The jbd2 buffer_head that has just been allocated for some
> + * use in the journal. jbd2_bh may not be NULL.
> + *
> + * Returns 0 on sucess or negative on failure
> + */
> +int jbd2_record_pair(journal_t *journal,
> + struct buffer_head *vfs_bh,
> + struct buffer_head *jbd2_bh){
> +
> + struct page *page;
> + struct buffer_head *bh;
> + struct buffer_head *head;
> + pgoff_t index;
> + struct jbd2_blk_pair *b_pair = NULL;
> + struct list_head *cur;
> + unsigned long long vfs_lblk = 0;
> + unsigned long long vfs_pblk = 0;
> + struct inode *vfs_inode = NULL;
> +
> + /* If we have the vfs bh, figure out the logical block offset */
> + if (vfs_bh) {
> + page = vfs_bh->b_page;
> + vfs_inode = page->mapping->host;
> + vfs_pblk = vfs_bh->b_blocknr;
> + index = page->index;
> +
> + /* Find the block offset of the page */
> + vfs_lblk = index <<
> + (PAGE_CACHE_SHIFT - vfs_inode->i_sb->s_blocksize_bits);
> +
> + /* Then add in the block offset of the bh in the page */
> + bh = page_buffers(page);
> + head = bh;
> + do {
> + if (bh == vfs_bh)
> + break;
> + bh = bh->b_this_page;
> + vfs_lblk++;
> + } while (bh != head);
> + }
> +
> + spin_lock(&journal->j_pair_lock);
> + /*
> + * Add the pair to the list. If there is already a pair
> + * for this journal block, just update it with the new vfs
> + * block info
> + */
> + list_for_each(cur, &journal->blk_pairs) {
> + b_pair = list_entry(cur, struct jbd2_blk_pair, list);
> + if (b_pair->jbd2_pblk == jbd2_bh->b_blocknr) {
> + b_pair->vfs_inode = vfs_inode;
> + b_pair->vfs_pblk = vfs_pblk;
> + b_pair->vfs_lblk = vfs_lblk;
> + b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
> +
> + break;
> + } else
> + b_pair = NULL;
> + }
> +
> + /*
> + * If the journal block was not found in the list,
> + * add a new pair to the list
> + */
> + if (!b_pair) {
> + b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS);
> + if (!b_pair) {
> + spin_unlock(&journal->j_pair_lock);
> + return -ENOMEM;
> + }
> + b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
> + b_pair->vfs_inode = vfs_inode;
> + b_pair->vfs_pblk = vfs_pblk;
> + b_pair->vfs_lblk = vfs_lblk;
> +
> + list_add(&b_pair->list, &journal->blk_pairs);
> + }
> +
> + spin_unlock(&journal->j_pair_lock);
> + return 0;
> +}
>
> /*
> * jbd2_journal_write_metadata_buffer: write a metadata buffer to the journal.
> @@ -739,7 +833,13 @@ struct journal_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
> lock_buffer(bh);
> memset(bh->b_data, 0, journal->j_blocksize);
> set_buffer_uptodate(bh);
> + err = jbd2_record_pair(journal, NULL, bh);
> +
> unlock_buffer(bh);
> +
> + if (err)
> + return NULL;
> +
> BUFFER_TRACE(bh, "return this buffer");
> return jbd2_journal_add_journal_head(bh);
> }
> @@ -895,9 +995,11 @@ static journal_t * journal_init_common (void)
> init_waitqueue_head(&journal->j_wait_commit);
> init_waitqueue_head(&journal->j_wait_updates);
> mutex_init(&journal->j_barrier);
> + INIT_LIST_HEAD(&journal->blk_pairs);
> mutex_init(&journal->j_checkpoint_mutex);
> spin_lock_init(&journal->j_revoke_lock);
> spin_lock_init(&journal->j_list_lock);
> + spin_lock_init(&journal->j_pair_lock);
> rwlock_init(&journal->j_state_lock);
>
> journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
> @@ -1361,6 +1463,8 @@ recovery_error:
> int jbd2_journal_destroy(journal_t *journal)
> {
> int err = 0;
> + struct list_head *cur;
> + struct jbd2_blk_pair *b_pair;
>
> /* Wait for the commit thread to wake up and die. */
> journal_kill_thread(journal);
> @@ -1405,6 +1509,14 @@ int jbd2_journal_destroy(journal_t *journal)
> iput(journal->j_inode);
> if (journal->j_revoke)
> jbd2_journal_destroy_revoke(journal);
> +
> + spin_lock(&journal->j_pair_lock);
> + list_for_each_prev(cur, &journal->blk_pairs) {
> + b_pair = list_entry(cur, struct jbd2_blk_pair, list);
> + kfree(b_pair);
> + }
> + spin_unlock(&journal->j_pair_lock);
> +
> kfree(journal->j_wbuf);
> kfree(journal);
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 38f307b..8b84b43 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -698,6 +698,19 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>
> #define JBD2_NR_BATCH 64
>
> +
> +/*
> + * List for keeping track of which vfs
> + * block a journal block contians.
> + */
> +struct jbd2_blk_pair {
> + struct inode *vfs_inode;
> + unsigned long long vfs_pblk;
> + unsigned long long vfs_lblk;
> + unsigned long long jbd2_pblk;
> + struct list_head list;
> +};
> +
> /**
> * struct journal_s - The journal_s type is the concrete type associated with
> * journal_t.
> @@ -1002,6 +1015,9 @@ struct journal_s
> * superblock pointer here
> */
> void *j_private;
> +
> + spinlock_t j_pair_lock;
> + struct list_head blk_pairs;
> };
>
> /*
> @@ -1080,6 +1096,11 @@ jbd2_journal_write_metadata_buffer(transaction_t *transaction,
> struct journal_head **jh_out,
> unsigned long long blocknr);
>
> +extern int jbd2_record_pair(journal_t *journal,
> + struct buffer_head *vfs_bh,
> + struct buffer_head *jbd2_bh);
> +
> +
> /* Transaction locking */
> extern void __wait_on_journal (journal_t *);
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/07/2011 11:35 AM, Darrick J. Wong wrote:
> 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.
I agree.
> 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.)
It looks like a useful feature. Though I would be wary of wiring this in
the journaling layer. Mainly for performance reasons.
In ocfs2, we log the truncated bits to a node specific system file called
truncate_log. These bits are flushed to the global bitmap periodically
by a queued task. We do this because taking a cluster lock on the global
bitmap is very expensive.
If I were doing this, I would extend this scheme to handle secure deletes.
The queued task would zero out the clusters before clearing the bits
in the global bitmap.
On 10/7/11 1:35 PM, 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.
And what about directory data? Those would appear to remain in the
journal at least... And xattrs?
#!/bin/bash
rm -f testsecdel
truncate --size 256m testsecdel
mkfs.ext4 -F testsecdel &>/dev/null
mount -o loop testsecdel mnt/
echo securedata > mnt/securefilename
setfattr -n user.securexattrname -v securexattrvalue mnt/securefilename
LONGATTR=`for I in 1 2 3 4 5 6 7 8 9 0; do echo -n veryveryveryveryveryveryverylongsecurexattrvalue; done`
setfattr -n user.longsecurexattrname -v $LONGATTR mnt/securefilename
sync
rm -f mnt/securefilename
umount mnt
strings testsecdel
yields:
/mnt/test2/mnt
lost+found
securexattrname
Ylongsecurexattrname
mselinux
veryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvaluesecurexattrvalueunconfined_u:object_r:file_t:s0
lost+found
securefilename
/mnt/test2/mnt
(this was with ext4.ko hacked to always enable secure delete)
-Eric
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<[email protected]>
>> ---
>> :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
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.
>> + if ((b_pair->vfs_inode == inode)&&
>> + (b_pair->vfs_lblk>= first_block)&&
>> + (b_pair->vfs_lblk< last_block)) {
>> +
>> + /*
>> + * It is likely that the journal blocks will be
>> + * consecutive, so lets try to secure delete them
>> + * in ranges
>> + */
>> + if (jbd2_pblk_count == 0) {
>> + /*
>> + * If there are no blocks in our range,
>> + * then this one will be the first
>> + */
>> + jbd2_pblk_start = b_pair->jbd2_pblk;
>> + jbd2_pblk_count = 1;
>> + } else if ((b_pair->jbd2_pblk) ==
>> + (jbd2_pblk_start + jbd2_pblk_count)) {
>> + /*
>> + * If this journal block is physically
>> + * consecutive, then just increase the range
>> + */
>> + jbd2_pblk_count++;
>> + } else if ((b_pair->jbd2_pblk) ==
>> + (jbd2_pblk_start - 1)&&
>> + jbd2_pblk_start> 0) {
>> + /*
>> + * If this journal block is physically
>> + * consecutive (from the start of the
>> + * range), just increase the range from the
>> + * other end.
>> + */
>> + jbd2_pblk_count++;
>> + jbd2_pblk_start--;
>> + } else {
>> + /*
>> + * If the block was not consecutive, secure
>> + * delete the range, and restart the current
>> + * range
>> + */
>> +
>> + err = ext4_secure_delete_pblks(journal->j_inode,
>> + jbd2_pblk_start, jbd2_pblk_count);
>> + if (err)
>> + goto out;
>> + jbd2_pblk_start = b_pair->jbd2_pblk;
>> + jbd2_pblk_count = 1;
>> + }
>> + }
>> + }
>> +
>> + /* Secure delete any blocks still in our range */
>> + if (jbd2_pblk_count> 0)
>> + err = ext4_secure_delete_pblks(journal->j_inode,
>> + jbd2_pblk_start, jbd2_pblk_count);
>> +
>> +out:
>> + spin_unlock(&journal->j_pair_lock);
>> + return err;
>> +}
>> +
>> struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
>> ext4_lblk_t block, int create, int *err)
>> {
>> @@ -3447,6 +3551,12 @@ void ext4_truncate(struct inode *inode)
>> else
>> ext4_ind_truncate(inode);
>>
>> + if (EXT4_I(inode)->i_flags& EXT4_SECRM_FL) {
>> + err = ext4_secure_delete_jblks(inode,
>> + inode->i_size>> EXT4_BLOCK_SIZE_BITS(inode->i_sb),
>> + EXT_MAX_BLOCKS);
>> + }
>> +
>> trace_ext4_truncate_exit(inode);
>> }
>>
>> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
>> index eef6979..2734e7b 100644
>> --- a/fs/jbd2/commit.c
>> +++ b/fs/jbd2/commit.c
>> @@ -593,6 +593,12 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>> jbd2_journal_abort(journal, flags);
>> continue;
>> }
>> +
>> + err = jbd2_record_pair(journal, jh2bh(jh), jh2bh(new_jh));
>> + if (err) {
>> + jbd2_journal_abort(journal, flags);
>> + continue;
>> + }
>> set_bit(BH_JWrite,&jh2bh(new_jh)->b_state);
>> wbuf[bufs++] = jh2bh(new_jh);
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index f24df13..6dd8af7 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -248,6 +248,100 @@ static void journal_kill_thread(journal_t *journal)
>> }
>> write_unlock(&journal->j_state_lock);
>> }
>> +/*
>> + * jbd2_record_pair()
>> + * Updates the journals list of block pairs to contain an new pair of journal
>> + * to vfs blocks. This list is an in memory record of the journals blocks to
>> + * help facilitate finding out what vfs blocks the journal blocks contain.
>> + * Secure delete uses this list to clear out stale journal blocks that may
>> + * still contain file data that needs to be secure deleted. This list is updated
>> + * when a vfs data block is written to the journal or when a descriptor block
>> + * is written to the journal.
>> + *
>> + * journal: The journal to update
>> + * vfs_bh: The vfs buffer_head that the jbd2_bh is journaling. This
>> + * may be NULL if jbd2_bh does not represent a vfs block.
>> + * jbd2_bh: The jbd2 buffer_head that has just been allocated for some
>> + * use in the journal. jbd2_bh may not be NULL.
>> + *
>> + * Returns 0 on sucess or negative on failure
>> + */
>> +int jbd2_record_pair(journal_t *journal,
>> + struct buffer_head *vfs_bh,
>> + struct buffer_head *jbd2_bh){
>> +
>> + struct page *page;
>> + struct buffer_head *bh;
>> + struct buffer_head *head;
>> + pgoff_t index;
>> + struct jbd2_blk_pair *b_pair = NULL;
>> + struct list_head *cur;
>> + unsigned long long vfs_lblk = 0;
>> + unsigned long long vfs_pblk = 0;
>> + struct inode *vfs_inode = NULL;
>> +
>> + /* If we have the vfs bh, figure out the logical block offset */
>> + if (vfs_bh) {
>> + page = vfs_bh->b_page;
>> + vfs_inode = page->mapping->host;
>> + vfs_pblk = vfs_bh->b_blocknr;
>> + index = page->index;
>> +
>> + /* Find the block offset of the page */
>> + vfs_lblk = index<<
>> + (PAGE_CACHE_SHIFT - vfs_inode->i_sb->s_blocksize_bits);
>> +
>> + /* Then add in the block offset of the bh in the page */
>> + bh = page_buffers(page);
>> + head = bh;
>> + do {
>> + if (bh == vfs_bh)
>> + break;
>> + bh = bh->b_this_page;
>> + vfs_lblk++;
>> + } while (bh != head);
>> + }
>> +
>> + spin_lock(&journal->j_pair_lock);
>> + /*
>> + * Add the pair to the list. If there is already a pair
>> + * for this journal block, just update it with the new vfs
>> + * block info
>> + */
>> + list_for_each(cur,&journal->blk_pairs) {
>> + b_pair = list_entry(cur, struct jbd2_blk_pair, list);
>> + if (b_pair->jbd2_pblk == jbd2_bh->b_blocknr) {
>> + b_pair->vfs_inode = vfs_inode;
>> + b_pair->vfs_pblk = vfs_pblk;
>> + b_pair->vfs_lblk = vfs_lblk;
>> + b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
>> +
>> + break;
>> + } else
>> + b_pair = NULL;
>> + }
>> +
>> + /*
>> + * If the journal block was not found in the list,
>> + * add a new pair to the list
>> + */
>> + if (!b_pair) {
>> + b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS);
>> + if (!b_pair) {
>> + spin_unlock(&journal->j_pair_lock);
>> + return -ENOMEM;
>> + }
>> + b_pair->jbd2_pblk = jbd2_bh->b_blocknr;
>> + b_pair->vfs_inode = vfs_inode;
>> + b_pair->vfs_pblk = vfs_pblk;
>> + b_pair->vfs_lblk = vfs_lblk;
>> +
>> + list_add(&b_pair->list,&journal->blk_pairs);
>> + }
>> +
>> + spin_unlock(&journal->j_pair_lock);
>> + return 0;
>> +}
>>
>> /*
>> * jbd2_journal_write_metadata_buffer: write a metadata buffer to the journal.
>> @@ -739,7 +833,13 @@ struct journal_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
>> lock_buffer(bh);
>> memset(bh->b_data, 0, journal->j_blocksize);
>> set_buffer_uptodate(bh);
>> + err = jbd2_record_pair(journal, NULL, bh);
>> +
>> unlock_buffer(bh);
>> +
>> + if (err)
>> + return NULL;
>> +
>> BUFFER_TRACE(bh, "return this buffer");
>> return jbd2_journal_add_journal_head(bh);
>> }
>> @@ -895,9 +995,11 @@ static journal_t * journal_init_common (void)
>> init_waitqueue_head(&journal->j_wait_commit);
>> init_waitqueue_head(&journal->j_wait_updates);
>> mutex_init(&journal->j_barrier);
>> + INIT_LIST_HEAD(&journal->blk_pairs);
>> mutex_init(&journal->j_checkpoint_mutex);
>> spin_lock_init(&journal->j_revoke_lock);
>> spin_lock_init(&journal->j_list_lock);
>> + spin_lock_init(&journal->j_pair_lock);
>> rwlock_init(&journal->j_state_lock);
>>
>> journal->j_commit_interval = (HZ * JBD2_DEFAULT_MAX_COMMIT_AGE);
>> @@ -1361,6 +1463,8 @@ recovery_error:
>> int jbd2_journal_destroy(journal_t *journal)
>> {
>> int err = 0;
>> + struct list_head *cur;
>> + struct jbd2_blk_pair *b_pair;
>>
>> /* Wait for the commit thread to wake up and die. */
>> journal_kill_thread(journal);
>> @@ -1405,6 +1509,14 @@ int jbd2_journal_destroy(journal_t *journal)
>> iput(journal->j_inode);
>> if (journal->j_revoke)
>> jbd2_journal_destroy_revoke(journal);
>> +
>> + spin_lock(&journal->j_pair_lock);
>> + list_for_each_prev(cur,&journal->blk_pairs) {
>> + b_pair = list_entry(cur, struct jbd2_blk_pair, list);
>> + kfree(b_pair);
>> + }
>> + spin_unlock(&journal->j_pair_lock);
>> +
>> kfree(journal->j_wbuf);
>> kfree(journal);
>>
>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>> index 38f307b..8b84b43 100644
>> --- a/include/linux/jbd2.h
>> +++ b/include/linux/jbd2.h
>> @@ -698,6 +698,19 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>>
>> #define JBD2_NR_BATCH 64
>>
>> +
>> +/*
>> + * List for keeping track of which vfs
>> + * block a journal block contians.
>> + */
>> +struct jbd2_blk_pair {
>> + struct inode *vfs_inode;
>> + unsigned long long vfs_pblk;
>> + unsigned long long vfs_lblk;
>> + unsigned long long jbd2_pblk;
>> + struct list_head list;
>> +};
>> +
>> /**
>> * struct journal_s - The journal_s type is the concrete type associated with
>> * journal_t.
>> @@ -1002,6 +1015,9 @@ struct journal_s
>> * superblock pointer here
>> */
>> void *j_private;
>> +
>> + spinlock_t j_pair_lock;
>> + struct list_head blk_pairs;
>> };
>>
>> /*
>> @@ -1080,6 +1096,11 @@ jbd2_journal_write_metadata_buffer(transaction_t *transaction,
>> struct journal_head **jh_out,
>> unsigned long long blocknr);
>>
>> +extern int jbd2_record_pair(journal_t *journal,
>> + struct buffer_head *vfs_bh,
>> + struct buffer_head *jbd2_bh);
>> +
>> +
>> /* Transaction locking */
>> extern void __wait_on_journal (journal_t *);
>>
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/07/2011 12:54 PM, Eric Sandeen wrote:
> On 10/7/11 1:35 PM, 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.
>
> And what about directory data? Those would appear to remain in the
> journal at least... And xattrs?
>
> #!/bin/bash
>
> rm -f testsecdel
> truncate --size 256m testsecdel
> mkfs.ext4 -F testsecdel&>/dev/null
> mount -o loop testsecdel mnt/
> echo securedata> mnt/securefilename
> setfattr -n user.securexattrname -v securexattrvalue mnt/securefilename
> LONGATTR=`for I in 1 2 3 4 5 6 7 8 9 0; do echo -n veryveryveryveryveryveryverylongsecurexattrvalue; done`
> setfattr -n user.longsecurexattrname -v $LONGATTR mnt/securefilename
> sync
>
> rm -f mnt/securefilename
> umount mnt
> strings testsecdel
>
> yields:
>
> /mnt/test2/mnt
> lost+found
> securexattrname
> Ylongsecurexattrname
> mselinux
> veryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvalueveryveryveryveryveryveryverylongsecurexattrvaluesecurexattrvalueunconfined_u:object_r:file_t:s0
> lost+found
> securefilename
> /mnt/test2/mnt
>
> (this was with ext4.ko hacked to always enable secure delete)
>
> -Eric
alrighty, I will need to figure out how to get those out of there too.
I will add this to my test case. Thx!
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<[email protected]>
>>> ---
>>> :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.
> 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
On 10/07/2011 11:07 AM, Darrick J. Wong wrote:
> On Fri, Oct 07, 2011 at 12:11:01AM -0700, Allison Henderson wrote:
>> This patch adds two new routines: ext4_secure_delete_pblks
>> and ext4_secure_delete_lblks.
>>
>> ext4_secure_delete_pblks() will write zeros to the specified
>> physical blocks or random data if the EXT4_SECRM_RANDOM_FL flag is
>> set. If the device supports secure discard, the secure
>> discard will be used instead. ext4_secure_delete_lblks handels walking
>
> handles
>
>> the logical blocks of a file and calling ext4_secure_delete_pblks()
>> as needed.
>>
>> Signed-off-by: Allison Henderson<[email protected]>
>> ---
>> v1->v2
>> Removed check for discard mount option and replaced with
>> check for secure discard and discard_zeroes_data
>>
>> Added BLKDEV_DISCARD_SECURE to the sb_issue_discard call
>>
>> v2->v3
>> Removed code for discard. A seperate patch will
>
> separate
>
>> be done to add that code in the block layer
>>
>> v3->v4
>> Discard code will be kept in the vfs layer. Code
>> for secure delete is now in its own function,
>> ext4_secure_delete_pblks and is called
>> by a new function ext4_secure_delete_lblks
>> before any blocks are released
>>
>> :100644 100644 5c9f88c... 34f82a1... M fs/ext4/ext4.h
>> :100644 100644 095c36f... 10180e3... M fs/ext4/ext4_extents.h
>> :100644 100644 57cf568... 40d4e50... M fs/ext4/extents.c
>> :100644 100644 9dc8c14... 0a526c4... M fs/ext4/inode.c
>> fs/ext4/ext4.h | 5 +
>> fs/ext4/ext4_extents.h | 2 +
>> fs/ext4/extents.c | 2 +-
>> fs/ext4/inode.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 204 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 5c9f88c..34f82a1 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -2220,6 +2220,11 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
>> struct ext4_map_blocks *map, int flags);
>> extern int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>> __u64 start, __u64 len);
>> +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);
>> +
>> /* move_extent.c */
>> extern int ext4_move_extents(struct file *o_filp, struct file *d_filp,
>> __u64 start_orig, __u64 start_donor,
>> diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
>> index 095c36f..10180e3 100644
>> --- a/fs/ext4/ext4_extents.h
>> +++ b/fs/ext4/ext4_extents.h
>> @@ -290,5 +290,7 @@ extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
>> struct ext4_ext_path *);
>> extern void ext4_ext_drop_refs(struct ext4_ext_path *);
>> extern int ext4_ext_check_inode(struct inode *inode);
>> +extern int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
>> + struct ext4_ext_cache *ex);
>> #endif /* _EXT4_EXTENTS */
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 57cf568..40d4e50 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -2034,7 +2034,7 @@ ext4_ext_put_gap_in_cache(struct inode *inode, struct ext4_ext_path *path,
>> *
>> * Return 0 if cache is invalid; 1 if the cache is valid
>> */
>> -static int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
>> +int ext4_ext_check_cache(struct inode *inode, ext4_lblk_t block,
>> struct ext4_ext_cache *ex){
>> struct ext4_ext_cache *cex;
>> struct ext4_sb_info *sbi;
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 9dc8c14..0a526c4 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -38,6 +38,7 @@
>> #include<linux/printk.h>
>> #include<linux/slab.h>
>> #include<linux/ratelimit.h>
>> +#include<linux/random.h>
>>
>> #include "ext4_jbd2.h"
>> #include "xattr.h"
>> @@ -713,6 +714,201 @@ static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
>> return 0;
>> }
>>
>> +
>> +/*
>> + * ext4_secure_delete_pblks
>> + *
>> + * Securely delete physical blocks.
>> + * If the devices supports secure discard,
>> + * blocks will be discarded. Otherwise
>> + * the blocks will be either zeroed or
>> + * randomized if the random secure delete
>> + * flag is on
>
> The fact that random secure delete produces zeroed blocks on discard devices is
> documented somewhere user-visible, right? Just in case someone actually
> depends on the randomizing.
At the moment no, the code is just the result of reviews and
brainstorming, but I can add in some documentation somewhere so that it
is more clear. Will catch the misspellings too, thx! :)
>
>> + * inode: The files inode
>> + * block: The physical block at which to start deleteing
>
> deleting
>
>> + * count: The number of blocks to delete
>> + *
>> + * Returns 0 on sucess or negative on error
>> + */
>> +int ext4_secure_delete_pblks(struct inode *inode, ext4_fsblk_t block,
>> + unsigned long count){
>> +
>> + struct fstrim_range range;
>> + ext4_fsblk_t iblock, last_block;
>> + struct buffer_head *bh;
>> + struct super_block *sb = inode->i_sb;
>> + struct request_queue *q = bdev_get_queue(sb->s_bdev);
>> + struct ext4_super_block *es = EXT4_SB(inode->i_sb)->s_es;
>> + int err = 0;
>> +
>> + last_block = block + count;
>> + /*
>> + * Check to see if the device supports secure discard,
>> + * And also that read after discard returns zeros
>> + */
>> + if (blk_queue_secdiscard(q)&& q->limits.discard_zeroes_data) {
>> + err = sb_issue_discard(sb, block, count,
>> + GFP_NOFS, BLKDEV_DISCARD_SECURE);
>> + if (err)
>> + goto zero_out;
>> +
>> + range.start = block;
>> + range.len = count;
>> + range.minlen = 1;
>> + err = ext4_trim_fs(sb,&range);
>> +
>> + if (err)
>> + goto zero_out;
>> +
>> + return 0;
>> + }
>> +
>> + if (EXT4_I(inode)->i_flags& EXT4_SECRM_RANDOM_FL) {
>> + for (iblock = block; iblock< last_block; iblock++) {
>> + bh = sb_getblk(sb, iblock);
>> + get_random_bytes(bh->b_data, bh->b_size);
>> + set_buffer_dirty(bh);
>> +
>> + sync_dirty_buffer(bh);
>> + if (buffer_req(bh)&& !buffer_uptodate(bh)) {
>> + es->s_last_error_block =
>> + cpu_to_le64(bh->b_blocknr);
>> + ext4_error_inode(inode, __func__,
>> + __LINE__, bh->b_blocknr,
>> + "IO error syncing itable block");
>
> itable block?
>
>> + err = -EIO;
>> + brelse(bh);
>> + goto zero_out;
>> + }
>> + brelse(bh);
>> + }
>> + return 0;
>> + }
>> +
>> +zero_out:
>> + return sb_issue_zeroout(sb, block, count, GFP_NOFS);
>> +
>> +}
>> +
>> +/*
>> + * ext4_secure_delete_lblks
>> + *
>> + * Secure deletes the data blocks of a file
>> + * starting at the given logical block
>> + *
>> + * @inode: The files inode
>> + * @first_block: Starting logical block
>> + * @count: The number of blocks to secure delete
>> + *
>> + * Returns 0 on sucess or negative on error
>> + */
>> +int ext4_secure_delete_lblks(struct inode *inode, ext4_lblk_t first_block,
>> + unsigned long count){
>> + handle_t *handle;
>> + struct ext4_map_blocks map;
>> + struct ext4_ext_cache cache_ex;
>> + ext4_lblk_t num_blocks, max_blocks = 0;
>> + ext4_lblk_t last_block = first_block + count;
>> + ext4_lblk_t iblock = first_block;
>> + int ret, credits, hole_len, err = 0;
>> +
>> + credits = ext4_writepage_trans_blocks(inode);
>> + handle = ext4_journal_start(inode, credits);
>> + if (IS_ERR(handle))
>> + return PTR_ERR(handle);
>> +
>> + down_write(&EXT4_I(inode)->i_data_sem);
>> + ext4_ext_invalidate_cache(inode);
>> + ext4_discard_preallocations(inode);
>> +
>> + /* Do not allow last_block to wrap when caller passes EXT_MAX_BLOCK */
>> + if (last_block< first_block)
>> + last_block = EXT_MAX_BLOCKS;
>> +
>> + while (iblock< last_block) {
>> + max_blocks = last_block - iblock;
>> + num_blocks = 1;
>> + memset(&map, 0, sizeof(map));
>> + map.m_lblk = iblock;
>> + map.m_len = max_blocks;
>> +
>> + if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
>> + ret = ext4_ext_map_blocks(handle, inode,&map, 0);
>> + else
>> + ret = ext4_ind_map_blocks(handle, inode,&map, 0);
>> +
>> + if (ret> 0) {
>> + err = ext4_secure_delete_pblks(inode,
>> + map.m_pblk, map.m_len);
>> + if (err)
>> + break;
>> + num_blocks = ret;
>> + } else if (ret == 0) {
>> + if (ext4_test_inode_flag(inode,
>> + EXT4_INODE_EXTENTS)) {
>> + /*
>> + * If map blocks could not find the block,
>> + * then it is in a hole. If the hole was
>> + * not already cached, then map blocks should
>> + * put it in the cache. So we can get the hole
>> + * out of the cache
>> + */
>> + memset(&cache_ex, 0, sizeof(cache_ex));
>> + if ((ext4_ext_check_cache(inode, iblock,
>> + &cache_ex))&& !cache_ex.ec_start) {
>> +
>> + /* The hole is cached */
>> + num_blocks = cache_ex.ec_block +
>> + cache_ex.ec_len - iblock;
>> +
>> + } else {
>> + /* reached EOF of extent file */
>> + break;
>> + }
>> + } else {
>> + hole_len = ext4_ind_hole_lookup(inode, iblock);
>> +
>> + if (hole_len> 0) {
>> + /* Skip over the hole */
>> + num_blocks = hole_len;
>> + } else if (hole_len == 0) {
>> + /* No hole, EOF reached */
>> + break;
>> + } else {
>> + /* Hole look up err */
>> + err = hole_len;
>> + break;
>> + }
>> + }
>> + } else {
>> + /* Map blocks error */
>> + err = ret;
>> + break;
>> + }
>> +
>> + if (num_blocks == 0) {
>> + /* This condition should never happen */
>> + ext_debug("Block lookup failed");
>> + err = -EIO;
>> + break;
>> + }
>> +
>> + iblock += num_blocks;
>> + }
>> +
>> + if (IS_SYNC(inode))
>> + ext4_handle_sync(handle);
>> +
>> + up_write(&EXT4_I(inode)->i_data_sem);
>> +
>> + inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
>> + ext4_mark_inode_dirty(handle, inode);
>> + ext4_journal_stop(handle);
>> +
>> + return err;
>> +}
>> +
>> struct buffer_head *ext4_bread(handle_t *handle, struct inode *inode,
>> ext4_lblk_t block, int create, int *err)
>> {
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
On 10/07/2011 10:47 AM, Darrick J. Wong wrote:
> On Fri, Oct 07, 2011 at 12:11:00AM -0700, Allison Henderson wrote:
>> This patch adds a new ext4_ind_hole_lookup function that
>> will be used to identify and skip over holes during a
>> secure delete operation
>>
>> Signed-off-by: Allison Henderson<[email protected]>
>> ---
>> :100644 100644 db54ce4... 5c9f88c... M fs/ext4/ext4.h
>> :100644 100644 0962642... 2fe9dfd... M fs/ext4/indirect.c
>> :100644 100644 c4da98a... 9dc8c14... M fs/ext4/inode.c
>> fs/ext4/ext4.h | 5 +++
>> fs/ext4/indirect.c | 2 +-
>> fs/ext4/inode.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 79 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index db54ce4..5c9f88c 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -2241,6 +2241,11 @@ extern int ext4_bio_write_page(struct ext4_io_submit *io,
>> /* mmp.c */
>> extern int ext4_multi_mount_protect(struct super_block *, ext4_fsblk_t);
>>
>> +/* indirects.c */
>> +extern int ext4_block_to_path(struct inode *inode,
>> + ext4_lblk_t i_block,
>> + ext4_lblk_t offsets[4], int *boundary);
>> +
>> /* BH_Uninit flag: blocks are allocated but uninitialized on disk */
>> enum ext4_state_bits {
>> BH_Uninit /* blocks are allocated but uninitialized on disk */
>> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
>> index 0962642..2fe9dfd 100644
>> --- a/fs/ext4/indirect.c
>> +++ b/fs/ext4/indirect.c
>> @@ -69,7 +69,7 @@ static inline void add_chain(Indirect *p, struct buffer_head *bh, __le32 *v)
>> * get there at all.
>> */
>>
>> -static int ext4_block_to_path(struct inode *inode,
>> +int ext4_block_to_path(struct inode *inode,
>> ext4_lblk_t i_block,
>> ext4_lblk_t offsets[4], int *boundary)
>> {
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index c4da98a..9dc8c14 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -640,6 +640,79 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
>> return bh;
>> }
>>
>> +/*
>> + * ext4_in_hole_lookup
>> + *
>> + * Returns the size of the hole located at the given block up to
>> + * the first boundry, or 0 if there is no hole. Returns negative on err.
>
> boundary
>
>> + */
>> +static int ext4_ind_hole_lookup(struct inode *inode, ext4_lblk_t block)
>> +{
>> + struct super_block *sb = inode->i_sb;
>> + struct buffer_head *bh;
>> + ext4_lblk_t offsets[4];
>> + ext4_lblk_t offset;
>> + __le32 *children;
>> + __le32 blk;
>> + int blocks_to_boundary = 0;
>> + int depth = 0;
>> + int hole_size = 0;
>> + unsigned int i, j;
>> +
>> + depth = ext4_block_to_path(inode, block, offsets,&blocks_to_boundary);
>> +
>> + if (depth == 0)
>> + return 0;
>> +
>> + offset = offsets[0];
>> + blk = EXT4_I(inode)->i_data[offset];
>> + if (blk == 0) {
>> + for (i = *offsets; i< EXT4_NDIR_BLOCKS; i++) {
>> + if (EXT4_I(inode)->i_data[i] == 0)
>> + hole_size++;
>> + else
>> + break;
>> + }
>> + return hole_size;
>> + }
>> +
>> +
>> + for (j = 1; j< depth; j++) {
>> + offset = offsets[j];
>> + bh = sb_getblk(sb, le32_to_cpu(blk));
>> + if (unlikely(!bh))
>> + return -EIO;
>> +
>> + if (!bh_uptodate_or_lock(bh)) {
>> + if (bh_submit_read(bh)< 0) {
>> + put_bh(bh);
>> + return -EIO;
>> + }
>> + /* validate block references */
>> + if (ext4_check_indirect_blockref(inode, bh)) {
>> + put_bh(bh);
>> + return -EIO;
>> + }
>> + }
>> +
>> + blk = ((__le32 *)bh->b_data)[offset];
>> + /* Reader: end */
>> + if (blk == 0) {
>> + children = (__le32 *)(bh->b_data);
>> + for (i = offset; i< offset +
>> + blocks_to_boundary; i++) {
>> + if (children[i] == 0)
>> + hole_size++;
>> + else
>> + break;
>> + }
>
> Do you need a put_bh here somewhere?
>
> --D
I think you're right, I will add that in somewhere, thx!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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<[email protected]>
>>>> ---
>>>> :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
On 10/07/2011 10:07 AM, Allison Henderson wrote:
> On 10/07/2011 08:21 AM, Andreas Dilger wrote:
>> On 2011-10-07, at 1:10 AM, Allison Henderson<[email protected]> wrote:
>>> Sorry for the delay in getting this next version out.
>>> I had some tasks to take care of, and now I'm picking up my
>>> secure delete work again. I'm still not quite done yet,
>>> but a lot has changed and I wanted to update people so that
>>> we have an idea of where its going. Currently the patch
>>> deals with data blocks, meta blocks, directory entries,
>>> journal blocks, and also provides an option for secure
>>> deleting with random data instead of just zeros.
>>> I'm also planning on adding some more patches to
>>> deal with inodes and also a mount option that turns
>>> on secure delete by default. Im still not quite done
>>> debugging, but Im just sending it out early to get
>>> some more eyes on it. Feed back appreciated! :)
>>>
>>> v3->v4
>>> Added a new file attribute flag EXT4_SECRM_RANDOM_FL
>>> This flag causes the secure delete operations to over write
>>> blocks with random data instead of zeros.
>>
>> Since inode flags are in short supply, and I suspect users that want this want it for all files, this should probably be a superblock flag?
>>
> That is a really good point. The first thing that comes to mind though would be the fact that it is a lot slower when the random flag is on especially for really big files. So that would be one case where I could imagine a user might want the ability to set different options per file. But since the flags are a limited resource, I can see where we may not want to spend it so quickly. I will see if maybe there is some way I can optimize it, but I would like to see more folks weigh in on this topic too.
>
I've had another suggestion for this come up, so I wanted to put it out
here to see what people think. Instead of a new flag, we could use a
flag that is mutually exclusive like the undelete flag, since a file
that is secure deleted, should not be undeletable. Would this be
something that people would be interested in doing?
>>> New function ext4_secure_delete_lblks added to walk
>>> data blocks and secure delete them before any blocks
>>> are removed.
>>>
>>> Meta blocks are secure deleted before they are
>>> released
>>>
>>> New function added to identify holes in ind files.
>>> Used by ext4_secure_delete_lblks to skip over holes
>>> during secure delete.
>>>
>>> Added another list in the journal structure to track
>>> journal blocks so that they can be secure deleted later.
>>>
>>> Added new ext4_secure_delete_jblks that secure deletes
>>> journal blocks that were used to journal the specified
>>> logical blocks
>>>
>>> Allison Henderson (7):
>>> ext4: Secure Delete: Add new EXT4_SECRM_RANDOM_FL flag
>>> ext4: Secure Delete: Add ext4_ind_hole_lookup function
>>> ext4: Secure Delete: Add secure delete functions
>>> ext4: Secure Delete: Secure delete file data
>>> ext4: Secure Delete: Secure delete directory entry
>>> ext4: Secure Delete: Secure delete meta data blocks
>>> ext4/jbd2: Secure Delete: Secure delete journal blocks
>>>
>>> fs/ext4/ext4.h | 28 +++-
>>> fs/ext4/ext4_extents.h | 2 +
>>> fs/ext4/extents.c | 21 +++-
>>> fs/ext4/indirect.c | 2 +-
>>> fs/ext4/inode.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> fs/ext4/mballoc.c | 8 +
>>> fs/ext4/namei.c | 64 +++++++-
>>> fs/jbd2/commit.c | 6 +
>>> fs/jbd2/journal.c | 112 ++++++++++++++
>>> include/linux/jbd2.h | 21 +++
>>> 10 files changed, 642 insertions(+), 13 deletions(-)
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
One quick question:
On Fri, 7 Oct 2011 00:11:05 -0700
Allison Henderson <[email protected]> wrote:
> + /* Secure delete any blocks still in our range */
> + if (jbd2_pblk_count > 0)
> + err = ext4_secure_delete_pblks(journal->j_inode,
> + jbd2_pblk_start, jbd2_pblk_count);
> +
> +out:
> + spin_unlock(&journal->j_pair_lock);
ext4_secure_delete_pblks() appears to do its job synchronously - it has
calls to things like sync_dirty_buffer() and such. How can you do that
while holding ->j_pair_lock?
Thanks,
jon
On Fri, 7 Oct 2011 00:11:05 -0700
Allison Henderson <[email protected]> wrote:
> + /*
> + * If the journal block was not found in the list,
> + * add a new pair to the list
> + */
> + if (!b_pair) {
> + b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS);
> + if (!b_pair) {
> + spin_unlock(&journal->j_pair_lock);
> + return -ENOMEM;
> + }
Here too... that really needs to be GFP_ATOMIC.
I'm wondering, though...it looks like, over a short period of time, you
will create an unordered linked list containing one entry for every
physical block in the journal. You'll take a lock and search the whole
list for every block that is committed. Wouldn't it be better just to
have an array indexed by the journal logical block offset? Less memory,
faster access...? Or am I missing something fundamental here?
Thanks,
jon
On 10/10/2011 12:47 PM, Jonathan Corbet wrote:
> One quick question:
>
> On Fri, 7 Oct 2011 00:11:05 -0700
> Allison Henderson<[email protected]> wrote:
>
>> + /* Secure delete any blocks still in our range */
>> + if (jbd2_pblk_count> 0)
>> + err = ext4_secure_delete_pblks(journal->j_inode,
>> + jbd2_pblk_start, jbd2_pblk_count);
>> +
>> +out:
>> + spin_unlock(&journal->j_pair_lock);
>
> ext4_secure_delete_pblks() appears to do its job synchronously - it has
> calls to things like sync_dirty_buffer() and such. How can you do that
> while holding ->j_pair_lock?
>
> Thanks,
>
> jon
>
Hi Jon,
Well j_pair_lock is a lock I added to protect the new list of vfs
-> jbd2 block pairs. It is locked by the journal commit thread to
update the list when ever a journal block is modified. The above
code here is called by the same thread that performs a punch hole or
truncate operation, not the journal commit thread. So I'm not
immediately seeing why there would be any lock problems. Is there
another case I'm missing?
Allison
On 10/10/2011 01:00 PM, Jonathan Corbet wrote:
> On Fri, 7 Oct 2011 00:11:05 -0700
> Allison Henderson<[email protected]> wrote:
>
>> + /*
>> + * If the journal block was not found in the list,
>> + * add a new pair to the list
>> + */
>> + if (!b_pair) {
>> + b_pair = kzalloc(sizeof(struct jbd2_blk_pair), GFP_NOFS);
>> + if (!b_pair) {
>> + spin_unlock(&journal->j_pair_lock);
>> + return -ENOMEM;
>> + }
>
> Here too... that really needs to be GFP_ATOMIC.
>
> I'm wondering, though...it looks like, over a short period of time, you
> will create an unordered linked list containing one entry for every
> physical block in the journal. You'll take a lock and search the whole
> list for every block that is committed. Wouldn't it be better just to
> have an array indexed by the journal logical block offset? Less memory,
> faster access...? Or am I missing something fundamental here?
>
> Thanks,
>
> jon
Hi Jon,
I think that's a good suggestion. Initially I had made it a linked
list since most of the journals existing lists were implemented as
linked lists, but since this list ends up being a fixed size, an
array is a good optimization here. Thx!
Allison
On Mon, 10 Oct 2011 16:35:24 -0700
Allison Henderson <[email protected]> wrote:
> On 10/10/2011 12:47 PM, Jonathan Corbet wrote:
> > One quick question:
> >
> > On Fri, 7 Oct 2011 00:11:05 -0700
> > Allison Henderson<[email protected]> wrote:
> >
> >> + /* Secure delete any blocks still in our range */
> >> + if (jbd2_pblk_count> 0)
> >> + err = ext4_secure_delete_pblks(journal->j_inode,
> >> + jbd2_pblk_start, jbd2_pblk_count);
> >> +
> >> +out:
> >> + spin_unlock(&journal->j_pair_lock);
> >
> > ext4_secure_delete_pblks() appears to do its job synchronously - it has
> > calls to things like sync_dirty_buffer() and such. How can you do that
> > while holding ->j_pair_lock?
> >
> > Thanks,
> >
> > jon
> >
>
> Hi Jon,
>
> Well j_pair_lock is a lock I added to protect the new list of vfs
> -> jbd2 block pairs. It is locked by the journal commit thread to
> update the list when ever a journal block is modified. The above
> code here is called by the same thread that performs a punch hole or
> truncate operation, not the journal commit thread. So I'm not
> immediately seeing why there would be any lock problems. Is there
> another case I'm missing?
The problem is that ext4_secure_delete_pblks() can sleep, unless I've
misunderstood things very badly. That's not something you want to do
while holding a spinlock...
jon
On 10/10/2011 04:41 PM, Jonathan Corbet wrote:
> On Mon, 10 Oct 2011 16:35:24 -0700
> Allison Henderson<[email protected]> wrote:
>
>> On 10/10/2011 12:47 PM, Jonathan Corbet wrote:
>>> One quick question:
>>>
>>> On Fri, 7 Oct 2011 00:11:05 -0700
>>> Allison Henderson<[email protected]> wrote:
>>>
>>>> + /* Secure delete any blocks still in our range */
>>>> + if (jbd2_pblk_count> 0)
>>>> + err = ext4_secure_delete_pblks(journal->j_inode,
>>>> + jbd2_pblk_start, jbd2_pblk_count);
>>>> +
>>>> +out:
>>>> + spin_unlock(&journal->j_pair_lock);
>>>
>>> ext4_secure_delete_pblks() appears to do its job synchronously - it has
>>> calls to things like sync_dirty_buffer() and such. How can you do that
>>> while holding ->j_pair_lock?
>>>
>>> Thanks,
>>>
>>> jon
>>>
>>
>> Hi Jon,
>>
>> Well j_pair_lock is a lock I added to protect the new list of vfs
>> -> jbd2 block pairs. It is locked by the journal commit thread to
>> update the list when ever a journal block is modified. The above
>> code here is called by the same thread that performs a punch hole or
>> truncate operation, not the journal commit thread. So I'm not
>> immediately seeing why there would be any lock problems. Is there
>> another case I'm missing?
>
> The problem is that ext4_secure_delete_pblks() can sleep, unless I've
> misunderstood things very badly. That's not something you want to do
> while holding a spinlock...
>
> jon
>
Oh I see the concern now. Ok, I can put in a semaphore instead. Thx
for catching that. :)
Allison