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

Andy Whitcroft apw at canonical.com
Fri Mar 16 11:00:39 UTC 2012


On Fri, Mar 16, 2012 at 09:16:51AM +0000, Colin King wrote:
> From: Colin Ian King <colin.king at 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

Confirmed these two are in Natty as cherry-picks.

> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  fs/ecryptfs/inode.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 645da17..3c1dbc0 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -777,6 +777,9 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia,
>  		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);
> +
>  	/* Set up a fake ecryptfs file, this is used to interface with
>  	 * the file in the underlying filesystem so that the
>  	 * truncation has an effect there as well. */

Seems to do what it says.

Acked-by: Andy Whitcroft <apw at canonical.com>

BTW as we are wrapping up Maverick, is this serious enough to warrent
including in any final upload.  Data corruption sounds like a critical
issue.

-apw




More information about the kernel-team mailing list