diff mbox

[1/1] eCryptfs: Clear ECRYPTFS_NEW_FILE flag during truncate

Message ID 1331905426-5695-2-git-send-email-colin.king@canonical.com
State New
Headers show

Commit Message

Colin Ian King March 16, 2012, 1:43 p.m. UTC 
From: Tyler Hicks <tyhicks@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/745836

The ECRYPTFS_NEW_FILE crypt_stat flag is set upon creation of a new
eCryptfs file. When the flag is set, eCryptfs reads directly from the
lower filesystem when bringing a page up to date. This means that no
offset translation (for the eCryptfs file metadata in the lower file)
and no decryption is performed. The flag is cleared just before the
first write is completed (at the beginning of ecryptfs_write_begin()).

It was discovered that if a new file was created and then extended with
truncate, the ECRYPTFS_NEW_FILE flag was not cleared. If pages
corresponding to this file are ever reclaimed, any subsequent reads
would result in userspace seeing eCryptfs file metadata and encrypted
file contents instead of the expected decrypted file contents.

Data corruption is possible if the file is written to before the
eCryptfs directory is unmounted. The data written will be copied into
pages which have been read directly from the lower file rather than
zeroed pages, as would be expected after extending the file with
truncate.

This flag, and the functionality that used it, was removed in upstream
kernels in 2.6.39 with the following commits:

bd4f0fe8bb7c73c738e1e11bc90d6e2cf9c6e20e
fed8859b3ab94274c986cbdf7d27130e0545f02c

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/ecryptfs/inode.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Stefan Bader March 16, 2012, 2:36 p.m. UTC | #1 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 16.03.2012 14:43, Colin King wrote:
> From: Tyler Hicks <tyhicks@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/745836
> 
> The ECRYPTFS_NEW_FILE crypt_stat flag is set upon creation of a new 
> eCryptfs file. When the flag is set, eCryptfs reads directly from the lower
> filesystem when bringing a page up to date. This means that no offset
> translation (for the eCryptfs file metadata in the lower file) and no
> decryption is performed. The flag is cleared just before the first write is
> completed (at the beginning of ecryptfs_write_begin()).
> 
> It was discovered that if a new file was created and then extended with 
> truncate, the ECRYPTFS_NEW_FILE flag was not cleared. If pages 
> corresponding to this file are ever reclaimed, any subsequent reads would
> result in userspace seeing eCryptfs file metadata and encrypted file
> contents instead of the expected decrypted file contents.
> 
> Data corruption is possible if the file is written to before the eCryptfs
> directory is unmounted. The data written will be copied into pages which
> have been read directly from the lower file rather than zeroed pages, as
> would be expected after extending the file with truncate.
> 
> This flag, and the functionality that used it, was removed in upstream 
> kernels in 2.6.39 with the following commits:
> 
> bd4f0fe8bb7c73c738e1e11bc90d6e2cf9c6e20e 
> fed8859b3ab94274c986cbdf7d27130e0545f02c
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Signed-off-by: Colin Ian
> King <colin.king@canonical.com> --- fs/ecryptfs/inode.c |    4 ++++ 1 files
> changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index
> 1681c62..6e9b4a3 100644 --- a/fs/ecryptfs/inode.c +++
> b/fs/ecryptfs/inode.c @@ -790,7 +790,11 @@ static int truncate_upper(struct
> dentry *dentry, struct iattr *ia, lower_ia->ia_valid &= ~ATTR_SIZE; goto
> out; } + crypt_stat =
> &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat; +	if
> (crypt_stat->flags & ECRYPTFS_NEW_FILE) +		crypt_stat->flags &=
> ~(ECRYPTFS_NEW_FILE); + /* Switch on growing or shrinking file */ if
> (ia->ia_size > i_size) { char zero[] = { 0x00 };

If we care to do this in Maverick...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJPY0/aAAoJEOhnXe7L7s6jvkUQANLIeKBmfll686nmGVPPC8Ox
4kFhaqhOMOUe1gTy0fgFlYs9Iy8pkm+v2wimZ717fKGteU+YoCrmJsphLBAKYfaB
g6nPa8qOe0IbRV+olFy+EDiyzH/JHBT521xsAt7lcUO04eDqlK9A3/w6oYOyO0sF
onSflPqYyvaaKsVVwwUVoNPolrx8QciZZ2SCQn3S1V+smvdVMQYlyP5Akr/yc7lQ
Y1UeRBuvfuNUGXzaKt/sOVPrXYrSPu2Y0zNGUiWq+BTSUyMnvd48ua0iZ+8wegxm
PVlFu20bRpI1SLzOtZGflB5cidLEYSVr1r0cgzjkWP/VBEVr8vRZQhYi0KsRa30L
D35GBc0P4Akyy64tZ6HrnCbRTVi8ESa6ZqxTddjb0iOeBLT43ovFCGOA8LlPrm0f
fecB9CMIE1WtN/2C+HT1EINNMC4lDL6wTGfeom7esEV3P5vvGKjF3yMvi0uezxp5
jWO5zpKwRLLGzzsKlparzrhA8jfbcFgyWTscgUut9krNNf3ZrGYxsZm8wEFlYM2O
1zMqkF/4W/MEi1jIvhEjK4CrY8sBxVvc7VG5xmsDSSw/okoHDxPkQ4ief9/Es2vF
LYZ+GE2zNr/RtheZb1kjD+729JA5iGnqhqK6X9WLeFQZ386YcYIfzhzG/r6PJfKA
lHmfrdhWj3ihij4k7Qhn
=aLAP
-----END PGP SIGNATURE-----
Herton Ronaldo Krzesinski March 16, 2012, 2:50 p.m. UTC | #2 
On Fri, Mar 16, 2012 at 01:43:46PM +0000, Colin King wrote:
> From: Tyler Hicks <tyhicks@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/745836
> 
> The ECRYPTFS_NEW_FILE crypt_stat flag is set upon creation of a new
> eCryptfs file. When the flag is set, eCryptfs reads directly from the
> lower filesystem when bringing a page up to date. This means that no
> offset translation (for the eCryptfs file metadata in the lower file)
> and no decryption is performed. The flag is cleared just before the
> first write is completed (at the beginning of ecryptfs_write_begin()).
> 
> It was discovered that if a new file was created and then extended with
> truncate, the ECRYPTFS_NEW_FILE flag was not cleared. If pages
> corresponding to this file are ever reclaimed, any subsequent reads
> would result in userspace seeing eCryptfs file metadata and encrypted
> file contents instead of the expected decrypted file contents.
> 
> Data corruption is possible if the file is written to before the
> eCryptfs directory is unmounted. The data written will be copied into
> pages which have been read directly from the lower file rather than
> zeroed pages, as would be expected after extending the file with
> truncate.
> 
> This flag, and the functionality that used it, was removed in upstream
> kernels in 2.6.39 with the following commits:
> 
> bd4f0fe8bb7c73c738e1e11bc90d6e2cf9c6e20e
> fed8859b3ab94274c986cbdf7d27130e0545f02c
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Ack, this probably makes an final upload of maverick needed.

> ---
>  fs/ecryptfs/inode.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 1681c62..6e9b4a3 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -790,7 +790,11 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
>  		lower_ia->ia_valid &= ~ATTR_SIZE;
>  		goto out;
>  	}
> +
>  	crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
> +	if (crypt_stat->flags & ECRYPTFS_NEW_FILE)
> +		crypt_stat->flags &= ~(ECRYPTFS_NEW_FILE);
> +
>  	/* Switch on growing or shrinking file */
>  	if (ia->ia_size > i_size) {
>  		char zero[] = { 0x00 };
> -- 
> 1.7.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
diff mbox

Patch

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 1681c62..6e9b4a3 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -790,7 +790,11 @@  static int truncate_upper(struct dentry *dentry, struct iattr *ia,
 		lower_ia->ia_valid &= ~ATTR_SIZE;
 		goto out;
 	}
+
 	crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
+	if (crypt_stat->flags & ECRYPTFS_NEW_FILE)
+		crypt_stat->flags &= ~(ECRYPTFS_NEW_FILE);
+
 	/* Switch on growing or shrinking file */
 	if (ia->ia_size > i_size) {
 		char zero[] = { 0x00 };