[Fwd: SRU LP292429 Error when copying directory tree with Nautilus to ~/Private using ecryptfs]

Tue Nov 18 21:06:43 UTC 2008

On Tue, Nov 18, 2008 at 01:38:31PM -0700, Tim Gardner wrote:
> This patch is being proposed as an SRU to Intrepid, but I'm having a
> hard time reconciling the description with the actual function of
> the patch.
> 
> Allocating multiple scatterlist structures has nothing to do with
> alignment, but what it _does_ do is correctly consume the second S/G
> entry if the buffer passed to virt_to_scatterlist() crosses a page
> boundary, correct?

That is correct. The patch compensates for the fact that the key value
may not fit in a single page.

> It seems to me that the bug occurs if the virtual pages consumed by
> the buffer are not physically contiguous.
> 
> rtg

> To: kernel-team at lists.canonical.com
> Subject: SRU LP292429 Error when copying directory tree with Nautilus to
> 	~/Private using ecryptfs
> From: Jim Lieb <jim.lieb at canonical.com>
> Date: Mon, 17 Nov 2008 18:13:24 -0800
> 
> OriginalAuthor: Michael Halcrow <mhalcrow at us.ibm.com>
> Bug: #292429
> 
> On Wed, Nov 12, 2008 at 12:36:10PM -0600, Michael Halcrow wrote:
> > Looks like crypt_stat->key is not page-aligned on this older AMD
> > architecture. This is a legitimate bug in eCryptfs and needs to be
> > fixed upstream. I think I will just grab a page via page_alloc() to
> > use as a temporary buffer for the crypto API scatterlist ops.
> 
> On second thought, it might make more sense just to allocate a couple
> of scatterlist structs on the stack every time instead. See if this
> patch resolves the problem. It tests fine for me on my Intel
> processor, and I expect it will resolve the problem on the AMD
> architecture.
> 
> This patch is not in the upstream yet. We expect it may appear
> in a different form later.
> 
> Committer: Jim Lieb <lieb at canonical.com>
> Signed-off-by: Michael Halcrow <mhalcrow at us.ibm.com>
> Signed-off-by: Jim Lieb <jim.lieb at canonical.com>
> ---
>  fs/ecryptfs/keystore.c |   31 ++++++++++++++-----------------
>  1 files changed, 14 insertions(+), 17 deletions(-)
> 
> The raw diff is attached
> 
> Jim

> From 0bd3efbad5b618fc2894e0650854e81fc1cbaf85 Mon Sep 17 00:00:00 2001
> From: Jim Lieb <lieb at jim-laptop.home.sea-troll.net>
> Date: Fri, 14 Nov 2008 14:03:54 -0800
> Subject: [PATCH] UBUNTU: SAUCE: Error when copying directory tree with Nautilus to ~/Private using ecryptfs
> 
> OriginalAuthor: Michael Halcrow <mhalcrow at us.ibm.com>
> Bug: #292429
> 
> On Wed, Nov 12, 2008 at 12:36:10PM -0600, Michael Halcrow wrote:
> > Looks like crypt_stat->key is not page-aligned on this older AMD
> > architecture. This is a legitimate bug in eCryptfs and needs to be
> > fixed upstream. I think I will just grab a page via page_alloc() to
> > use as a temporary buffer for the crypto API scatterlist ops.
> 
> On second thought, it might make more sense just to allocate a couple
> of scatterlist structs on the stack every time instead. See if this
> patch resolves the problem. It tests fine for me on my Intel
> processor, and I expect it will resolve the problem on the AMD
> architecture.
> 
> This patch is not in the upstream yet. We expect it may appear
> in a different form later.
> 
> Committer: Jim Lieb <lieb at canonical.com>
> Signed-off-by: Michael Halcrow <mhalcrow at us.ibm.com>
> Signed-off-by: Jim Lieb <jim.lieb at canonical.com>
> ---
>  fs/ecryptfs/keystore.c |   31 ++++++++++++++-----------------
>  1 files changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index f5b76a3..59b9833 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -1037,17 +1037,14 @@ static int
>  decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
>  					 struct ecryptfs_crypt_stat *crypt_stat)
>  {
> -	struct scatterlist dst_sg;
> -	struct scatterlist src_sg;
> +	struct scatterlist dst_sg[2];
> +	struct scatterlist src_sg[2];
>  	struct mutex *tfm_mutex;
>  	struct blkcipher_desc desc = {
>  		.flags = CRYPTO_TFM_REQ_MAY_SLEEP
>  	};
>  	int rc = 0;
>  
> -	sg_init_table(&dst_sg, 1);
> -	sg_init_table(&src_sg, 1);
> -
>  	if (unlikely(ecryptfs_verbosity > 0)) {
>  		ecryptfs_printk(
>  			KERN_DEBUG, "Session key encryption key (size [%d]):\n",
> @@ -1066,8 +1063,8 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
>  	}
>  	rc = virt_to_scatterlist(auth_tok->session_key.encrypted_key,
>  				 auth_tok->session_key.encrypted_key_size,
> -				 &src_sg, 1);
> -	if (rc != 1) {
> +				 src_sg, 2);
> +	if (rc < 1 || rc > 2) {
>  		printk(KERN_ERR "Internal error whilst attempting to convert "
>  			"auth_tok->session_key.encrypted_key to scatterlist; "
>  			"expected rc = 1; got rc = [%d]. "
> @@ -1079,8 +1076,8 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
>  		auth_tok->session_key.encrypted_key_size;
>  	rc = virt_to_scatterlist(auth_tok->session_key.decrypted_key,
>  				 auth_tok->session_key.decrypted_key_size,
> -				 &dst_sg, 1);
> -	if (rc != 1) {
> +				 dst_sg, 2);
> +	if (rc < 1 || rc > 2) {
>  		printk(KERN_ERR "Internal error whilst attempting to convert "
>  			"auth_tok->session_key.decrypted_key to scatterlist; "
>  			"expected rc = 1; got rc = [%d]\n", rc);
> @@ -1096,7 +1093,7 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
>  		rc = -EINVAL;
>  		goto out;
>  	}
> -	rc = crypto_blkcipher_decrypt(&desc, &dst_sg, &src_sg,
> +	rc = crypto_blkcipher_decrypt(&desc, dst_sg, src_sg,
>  				      auth_tok->session_key.encrypted_key_size);
>  	mutex_unlock(tfm_mutex);
>  	if (unlikely(rc)) {
> @@ -1541,8 +1538,8 @@ write_tag_3_packet(char *dest, size_t *remaining_bytes,
>  	size_t i;
>  	size_t encrypted_session_key_valid = 0;
>  	char session_key_encryption_key[ECRYPTFS_MAX_KEY_BYTES];
> -	struct scatterlist dst_sg;
> -	struct scatterlist src_sg;
> +	struct scatterlist dst_sg[2];
> +	struct scatterlist src_sg[2];
>  	struct mutex *tfm_mutex = NULL;
>  	u8 cipher_code;
>  	size_t packet_size_length;
> @@ -1621,8 +1618,8 @@ write_tag_3_packet(char *dest, size_t *remaining_bytes,
>  		ecryptfs_dump_hex(session_key_encryption_key, 16);
>  	}
>  	rc = virt_to_scatterlist(crypt_stat->key, key_rec->enc_key_size,
> -				 &src_sg, 1);
> -	if (rc != 1) {
> +				 src_sg, 2);
> +	if (rc < 1 || rc > 2) {
>  		ecryptfs_printk(KERN_ERR, "Error generating scatterlist "
>  				"for crypt_stat session key; expected rc = 1; "
>  				"got rc = [%d]. key_rec->enc_key_size = [%d]\n",
> @@ -1631,8 +1628,8 @@ write_tag_3_packet(char *dest, size_t *remaining_bytes,
>  		goto out;
>  	}
>  	rc = virt_to_scatterlist(key_rec->enc_key, key_rec->enc_key_size,
> -				 &dst_sg, 1);
> -	if (rc != 1) {
> +				 dst_sg, 2);
> +	if (rc < 1 || rc > 2) {
>  		ecryptfs_printk(KERN_ERR, "Error generating scatterlist "
>  				"for crypt_stat encrypted session key; "
>  				"expected rc = 1; got rc = [%d]. "
> @@ -1653,7 +1650,7 @@ write_tag_3_packet(char *dest, size_t *remaining_bytes,
>  	rc = 0;
>  	ecryptfs_printk(KERN_DEBUG, "Encrypting [%d] bytes of the key\n",
>  			crypt_stat->key_size);
> -	rc = crypto_blkcipher_encrypt(&desc, &dst_sg, &src_sg,
> +	rc = crypto_blkcipher_encrypt(&desc, dst_sg, src_sg,
>  				      (*key_rec).enc_key_size);
>  	mutex_unlock(tfm_mutex);
>  	if (rc) {

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20081118/0e8156bd/attachment.sig>