SRU: Bug #242448 - Hardy ecryptfs can cause data loss

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

SRU: Bug #242448 - Hardy ecryptfs can cause data loss

Tim Gardner-6
SRU Justification:

Impact: ecryptfs can cause loss of data in certain scenarios

Patch Description: cherry-picked 2.6.25.y
d3e49afbb66109613c3474f2273f5830ac2dcb09. The page decrypt calls in
ecryptfs_write() are both pointless and buggy. Pointless because
ecryptfs_get_locked_page() has already brought the page up to date, and
buggy because prior mmap writes will just be blown away by the decrypt
call. This patch also removes the declaration of a now-nonexistent
function ecryptfs_write_zeros().

Patch:
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=commit;h=b85452a23dcdeddc4a4ae1816501e80a16bb14a8

Test Case: see Bug decscription

--
Tim Gardner [hidden email]

From b85452a23dcdeddc4a4ae1816501e80a16bb14a8 Mon Sep 17 00:00:00 2001
From: Michael Halcrow <[hidden email]>
Date: Fri, 6 Jun 2008 18:43:31 +0000
Subject: [PATCH] eCryptfs: remove unnecessary page decrypt call

upstream commit: d3e49afbb66109613c3474f2273f5830ac2dcb09

The page decrypt calls in ecryptfs_write() are both pointless and buggy.
Pointless because ecryptfs_get_locked_page() has already brought the page
up to date, and buggy because prior mmap writes will just be blown away by
the decrypt call.

This patch also removes the declaration of a now-nonexistent function
ecryptfs_write_zeros().

Thanks to Eric Sandeen and David Kleikamp for helping to track this
down.

Eric said:

   fsx w/ mmap dies quickly ( < 100 ops) without this, and survives
   nicely (to millions of ops+) with it in place.

Signed-off-by: Michael Halcrow <[hidden email]>
Cc: Eric Sandeen <[hidden email]>
Cc: Dave Kleikamp <[hidden email]>
Cc: <[hidden email]>
Signed-off-by: Andrew Morton <[hidden email]>
Signed-off-by: Linus Torvalds <[hidden email]>
[chrisw: backport to 2.6.25.5]
Signed-off-by: Chris Wright <[hidden email]>
[tim.gardner: backport to 2.6.24]
Signed-off-by: Tim Gardner <[hidden email]>
---
 fs/ecryptfs/ecryptfs_kernel.h |    2 --
 fs/ecryptfs/read_write.c      |   22 ----------------------
 2 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index ce7a5d4..b2b4ce4 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -629,8 +629,6 @@ int ecryptfs_get_tfm_and_mutex_for_cipher_name(struct crypto_blkcipher **tfm,
 int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key,
       struct ecryptfs_auth_tok **auth_tok,
       char *sig);
-int ecryptfs_write_zeros(struct file *file, pgoff_t index, int start,
- int num_zeros);
 void ecryptfs_lower_offset_for_extent(loff_t *offset, loff_t extent_num,
       struct ecryptfs_crypt_stat *crypt_stat);
 int ecryptfs_write_lower(struct inode *ecryptfs_inode, char *data,
diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c
index 948f576..1c8a354 100644
--- a/fs/ecryptfs/read_write.c
+++ b/fs/ecryptfs/read_write.c
@@ -157,20 +157,6 @@ int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset,
        ecryptfs_page_idx, rc);
  goto out;
  }
- if (start_offset_in_page) {
- /* Read in the page from the lower
- * into the eCryptfs inode page cache,
- * decrypting */
- rc = ecryptfs_decrypt_page(ecryptfs_page);
- if (rc) {
- printk(KERN_ERR "%s: Error decrypting "
-       "page; rc = [%d]\n",
-       __FUNCTION__, rc);
- ClearPageUptodate(ecryptfs_page);
- page_cache_release(ecryptfs_page);
- goto out;
- }
- }
  ecryptfs_page_virt = kmap_atomic(ecryptfs_page, KM_USER0);
 
  /*
@@ -348,14 +334,6 @@ int ecryptfs_read(char *data, loff_t offset, size_t size,
        ecryptfs_page_idx, rc);
  goto out;
  }
- rc = ecryptfs_decrypt_page(ecryptfs_page);
- if (rc) {
- printk(KERN_ERR "%s: Error decrypting "
-       "page; rc = [%d]\n", __FUNCTION__, rc);
- ClearPageUptodate(ecryptfs_page);
- page_cache_release(ecryptfs_page);
- goto out;
- }
  ecryptfs_page_virt = kmap_atomic(ecryptfs_page, KM_USER0);
  memcpy((data + data_offset),
        ((char *)ecryptfs_page_virt + start_offset_in_page),
--
1.5.4.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: SRU: Bug #242448 - Hardy ecryptfs can cause data loss

Ben Collins-4
On Mon, 2008-06-23 at 13:50 -0600, Tim Gardner wrote:

> SRU Justification:
>
> Impact: ecryptfs can cause loss of data in certain scenarios
>
> Patch Description: cherry-picked 2.6.25.y
> d3e49afbb66109613c3474f2273f5830ac2dcb09. The page decrypt calls in
> ecryptfs_write() are both pointless and buggy. Pointless because
> ecryptfs_get_locked_page() has already brought the page up to date, and
> buggy because prior mmap writes will just be blown away by the decrypt
> call. This patch also removes the declaration of a now-nonexistent
> function ecryptfs_write_zeros().
>
> Patch:
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=commit;h=b85452a23dcdeddc4a4ae1816501e80a16bb14a8
>
> Test Case: see Bug decscription

Doesn't look like it will change ABI, so ACK from me.


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: SRU: Bug #242448 - Hardy ecryptfs can cause data loss

Stefan Bader-2
In reply to this post by Tim Gardner-6
Tim Gardner wrote:

> SRU Justification:
>
> Impact: ecryptfs can cause loss of data in certain scenarios
>
> Patch Description: cherry-picked 2.6.25.y
> d3e49afbb66109613c3474f2273f5830ac2dcb09. The page decrypt calls in
> ecryptfs_write() are both pointless and buggy. Pointless because
> ecryptfs_get_locked_page() has already brought the page up to date, and
> buggy because prior mmap writes will just be blown away by the decrypt
> call. This patch also removes the declaration of a now-nonexistent
> function ecryptfs_write_zeros().
>
> Patch:
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=commit;h=b85452a23dcdeddc4a4ae1816501e80a16bb14a8
>
> Test Case: see Bug decscription
>
>
Ack

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team