linux-kernel - [PATCH 1/3] eCryptfs: Write bug for non-ecryptfs file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1329410210-2712-1-git-send-email-liwang@nudt.edu.cn>
Date:	Fri, 17 Feb 2012 00:36:48 +0800
From:	Li Wang <liwang@...t.edu.cn>
To:	Tyler Hicks <tyhicks@...onical.com>
Cc:	<ecryptfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Li Wang <liwang@...t.edu.cn>,
	Yunchuan Wen <wenyunchuan@...inos.com.cn>
Subject: [PATCH 1/3] eCryptfs: Write bug for non-ecryptfs file

The following code segment in ecryptfs_write_begin(mmap.c) is problematic,

1  if (!PageUptodate(page)) {
2	if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
3		...
4	} else if(crypt_stat->flags & ECRYPTFS_VIEW_AS_ENCRYPTED) {
5		...
6	} else {
7		if (prev_page_end_size >= i_size_read(page->mapping->host)) {
8			zero_user(page, 0, PAGE_CACHE_SIZE);
9		}
10		...
11	}
12	...
13      /* Writing to a new page, and creating a small hole from start
14       * of page? Zero it out. */
15	if ((i_size_read(mapping->host)==prev_page_end_size) && (pos!=0))
16		zero_user(page, 0, PAGE_CACHE_SIZE);

1 The check on 'pos!=0' for the IF statement in line 15 implies that while
  pos==0, the page needs not be zeroed. Unfortunately, that is not true,
  suppose an empty non-ecryptfs file has been created beforehand, currently the
  writter want to write at pos==0 with a length of m bytes, m<PAGE_CACHE_SIZE,
  now the page will not be zeroed, and set to Uptodate! Then the data within
  (m,PAGE_CACHE_SIZE) read by any reader will be undefined.
2 Even remove 'pos!=0', there is further more a case which does not be taken
  into account,
  if a file satisfies (i_size_read(mapping->host) > prev_page_end_size), and is
  non-ecryptfs file, then the page is not zeroed, but the data within
  (i_size_read(mapping->host), (prev_page_end_size+1)<<PAGE_CACHE_SHIFT) need
  be zeroed as well.
3 For ecryptfs file, if satisfies (prev_page_end_size == i_size_read(page->
  mapping->host) && pos!=0, this page will be double zeroed.

This patch solves the above problem.

Signed-off-by: Li Wang <liwang@...t.edu.cn>
Signed-off-by: Yunchuan Wen <wenyunchuan@...inos.com.cn>
---
 fs/ecryptfs/mmap.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 10ec695..27c0da8 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -308,6 +308,8 @@ static int ecryptfs_write_begin(struct file *file,
 			&ecryptfs_inode_to_private(mapping->host)->crypt_stat;

 		if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
+			if ((i_size_read(mapping->host) >> PAGE_CACHE_SHIFT) <= index)
+				zero_user(page, 0, PAGE_CACHE_SIZE);
 			rc = ecryptfs_read_lower_page_segment(
 				page, index, 0, PAGE_CACHE_SIZE, mapping->host);
 			if (rc) {
@@ -379,11 +381,6 @@ static int ecryptfs_write_begin(struct file *file,
 			}
 		}
 	}
-	/* Writing to a new page, and creating a small hole from start
-	 * of page?  Zero it out. */
-	if ((i_size_read(mapping->host) == prev_page_end_size)
-	    && (pos != 0))
-		zero_user(page, 0, PAGE_CACHE_SIZE);
 out:
 	if (unlikely(rc)) {
 		unlock_page(page);
-- 
1.7.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/