linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: brookxu <brookxu.cn@gmail.com>
To: tytso@mit.edu, adilger.kernel@dilger.ca,
	linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] ext4: mark extents index blocks as dirty to avoid information leakage
Date: Tue, 3 Mar 2020 16:51:06 +0800	[thread overview]
Message-ID: <e988a1db-3105-07a0-6399-38af80656af1@gmail.com> (raw)

From: Chunguang Xu <brookxu@tencent.com>

In the scene of deleting a file, the physical block information in the
extent will be cleared to 0, the buffer_head contains these extents is
marked as dirty, and then managed by jbd2, which will clear the
buffer_head's dirty flag by clear_buffer_dirty. However, when the entire
extent block is deleted, it is revoked from the jbd2, but  the extents
block is not redirtied.

Not quite reasonable here, for the following concerns:

1. This has the risk of information leakage and leads to an interesting
phenomenon that deleting the entire file is no more secure than truncate
to 1 byte, because the whole extents physical block clear to zero in cache
will never written back as the page is not redirtied.

2. For large files, the number of index block is usually very small.
Ignoring index pages not get much more benefit in IO performance. But if
we remark the page as dirty, the page is then written back by the system
writeback mechanism asynchronously with little performance impact. As a
result, the risk of information leakage can be avoided. At least not wrose
than truncate file length to 1 byte

Therefore, when the index block is released, we need to remark its page
as dirty, so that the index block on the disk will be updated and the
data is more security.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/ext4.h      | 1 +
 fs/ext4/ext4_jbd2.c | 8 ++++++++
 fs/ext4/extents.c   | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 61b37a0..f9a4d97 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -644,6 +644,7 @@ enum {
 #define EXT4_FREE_BLOCKS_NOFREE_FIRST_CLUSTER    0x0010
 #define EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER    0x0020
 #define EXT4_FREE_BLOCKS_RERESERVE_CLUSTER      0x0040
+#define EXT4_FREE_BLOCKS_METADATA_INDEX        0x0080
 
 /*
  * ioctl commands
diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index 1f53d64..7974c62 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -275,7 +275,15 @@ int __ext4_forget(const char *where, unsigned int line, handle_t *handle,
         ext4_set_errno(inode->i_sb, -err);
         __ext4_abort(inode->i_sb, where, line,
                "error %d when attempting revoke", err);
+    } else  {
+        /*
+         * we dirtied index block to ensure that related changes to
+         * the index block will be stored to disk.
+         */
+        if (is_metadata & EXT4_FREE_BLOCKS_METADATA_INDEX)
+            mark_buffer_dirty(bh);
     }
+
     BUFFER_TRACE(bh, "exit");
     return err;
 }
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 954013d..2ee0df0 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2431,7 +2431,8 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
     trace_ext4_ext_rm_idx(inode, leaf);
 
     ext4_free_blocks(handle, inode, NULL, leaf, 1,
-             EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
+             EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET |
+             EXT4_FREE_BLOCKS_METADATA_INDEX);
 
     while (--depth >= 0) {
         if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr))
-- 
1.8.3.1


             reply	other threads:[~2020-03-03  8:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-03  8:51 brookxu [this message]
2020-03-06  3:17 ` [PATCH] ext4: mark extents index blocks as dirty to avoid information leakage brookxu
2020-03-12 14:46 ` Theodore Y. Ts'o
2020-03-16  8:01   ` brookxu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e988a1db-3105-07a0-6399-38af80656af1@gmail.com \
    --to=brookxu.cn@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).