Re: [PATCH 3/11] eCryptfs: read_write.c routines

[Michael Halcrow <mhalcrow@us.ibm.com>]

On Wed, Sep 19, 2007 at 10:38:50PM -0700, Andrew Morton wrote:
> > +	offset = (page_for_lower->index << PAGE_CACHE_SHIFT) + offset_in_page;
> 
> bug.  You need to cast page.index to loff_t before shifting.
> 
> I'd fix it on the spot, but this would be a good time to review the
> whole patchset and perhaps the whole fs for this easy-to-do,
> hard-to-find bug.

Update data types and add casts in order to avoid potential overflow
issues.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 5d27cf9..4bf1a95 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -149,7 +149,7 @@ out:
  * ecryptfs_derive_iv
  * @iv: destination for the derived iv vale
  * @crypt_stat: Pointer to crypt_stat struct for the current inode
- * @offset: Offset of the page whose's iv we are to derive
+ * @offset: Offset of the extent whose IV we are to derive
  *
  * Generate the initialization vector from the given root IV and page
  * offset.
@@ -157,7 +157,7 @@ out:
  * Returns zero on success; non-zero on error.
  */
 static int ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat,
-			      pgoff_t offset)
+			      loff_t offset)
 {
 	int rc = 0;
 	char dst[MD5_DIGEST_SIZE];
@@ -173,7 +173,7 @@ static int ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat,
 	 * hashing business. -Halcrow */
 	memcpy(src, crypt_stat->root_iv, crypt_stat->iv_bytes);
 	memset((src + crypt_stat->iv_bytes), 0, 16);
-	snprintf((src + crypt_stat->iv_bytes), 16, "%ld", offset);
+	snprintf((src + crypt_stat->iv_bytes), 16, "%lld", offset);
 	if (unlikely(ecryptfs_verbosity > 0)) {
 		ecryptfs_printk(KERN_DEBUG, "source:\n");
 		ecryptfs_dump_hex(src, (crypt_stat->iv_bytes + 16));
@@ -384,11 +384,11 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page,
 				   struct page *page,
 				   unsigned long extent_offset)
 {
-	unsigned long extent_base;
+	loff_t extent_base;
 	char extent_iv[ECRYPTFS_MAX_IV_BYTES];
 	int rc;
 
-	extent_base = (page->index
+	extent_base = (((loff_t)page->index)
 		       * (PAGE_CACHE_SIZE / crypt_stat->extent_size));
 	rc = ecryptfs_derive_iv(extent_iv, crypt_stat,
 				(extent_base + extent_offset));
@@ -492,8 +492,9 @@ int ecryptfs_encrypt_page(struct page *page)
 			goto out;
 		}
 		ecryptfs_lower_offset_for_extent(
-			&offset, ((page->index * (PAGE_CACHE_SIZE
-						  / crypt_stat->extent_size))
+			&offset, ((((loff_t)page->index)
+				   * (PAGE_CACHE_SIZE
+				      / crypt_stat->extent_size))
 				  + extent_offset), crypt_stat);
 		rc = ecryptfs_write_lower(ecryptfs_inode, enc_extent_virt,
 					  offset, crypt_stat->extent_size);
@@ -515,11 +516,11 @@ static int ecryptfs_decrypt_extent(struct page *page,
 				   struct page *enc_extent_page,
 				   unsigned long extent_offset)
 {
-	unsigned long extent_base;
+	loff_t extent_base;
 	char extent_iv[ECRYPTFS_MAX_IV_BYTES];
 	int rc;
 
-	extent_base = (page->index
+	extent_base = (((loff_t)page->index)
 		       * (PAGE_CACHE_SIZE / crypt_stat->extent_size));
 	rc = ecryptfs_derive_iv(extent_iv, crypt_stat,
 				(extent_base + extent_offset));
@@ -1320,7 +1321,7 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat,
 	while (current_header_page < header_pages) {
 		loff_t offset;
 
-		offset = (current_header_page << PAGE_CACHE_SHIFT);
+		offset = (((loff_t)current_header_page) << PAGE_CACHE_SHIFT);
 		if ((rc = ecryptfs_write_lower(ecryptfs_dentry->d_inode,
 					       page_virt, offset,
 					       PAGE_CACHE_SIZE))) {
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index c6a8a33..4eb09c1 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -127,7 +127,8 @@ ecryptfs_copy_up_encrypted_with_header(struct page *page,
 	int rc = 0;
 
 	while (extent_num_in_page < num_extents_per_page) {
-		loff_t view_extent_num = ((page->index * num_extents_per_page)
+		loff_t view_extent_num = ((((loff_t)page->index)
+					   * num_extents_per_page)
 					  + extent_num_in_page);
 
 		if (view_extent_num < crypt_stat->num_header_extents_at_front) {
@@ -418,7 +419,7 @@ static int ecryptfs_commit_write(struct file *file, struct page *page,
 				"index [0x%.16x])\n", page->index);
 		goto out;
 	}
-	pos = (page->index << PAGE_CACHE_SHIFT) + to;
+	pos = (((loff_t)page->index) << PAGE_CACHE_SHIFT) + to;
 	if (pos > i_size_read(ecryptfs_inode)) {
 		i_size_write(ecryptfs_inode, pos);
 		ecryptfs_printk(KERN_DEBUG, "Expanded file size to "
diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c
index ccd2599..272eaeb 100644
--- a/fs/ecryptfs/read_write.c
+++ b/fs/ecryptfs/read_write.c
@@ -87,7 +87,8 @@ int ecryptfs_write_lower_page_segment(struct inode *ecryptfs_inode,
 	loff_t offset;
 	int rc;
 
-	offset = (page_for_lower->index << PAGE_CACHE_SHIFT) + offset_in_page;
+	offset = ((((off_t)page_for_lower->index) << PAGE_CACHE_SHIFT)
+		  + offset_in_page);
 	virt = kmap(page_for_lower);
 	rc = ecryptfs_write_lower(ecryptfs_inode, virt, offset, size);
 	kunmap(page_for_lower);
@@ -117,7 +118,8 @@ int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset,
 {
 	struct page *ecryptfs_page;
 	char *ecryptfs_page_virt;
-	u64 ecryptfs_file_size = i_size_read(ecryptfs_file->f_dentry->d_inode);
+	loff_t ecryptfs_file_size =
+		i_size_read(ecryptfs_file->f_dentry->d_inode);
 	loff_t data_offset = 0;
 	loff_t pos;
 	int rc = 0;
@@ -277,7 +279,7 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs,
 	loff_t offset;
 	int rc;
 
-	offset = ((page_index << PAGE_CACHE_SHIFT) + offset_in_page);
+	offset = ((((loff_t)page_index) << PAGE_CACHE_SHIFT) + offset_in_page);
 	virt = kmap(page_for_ecryptfs);
 	rc = ecryptfs_read_lower(virt, offset, size, ecryptfs_inode);
 	kunmap(page_for_ecryptfs);
@@ -306,7 +308,8 @@ int ecryptfs_read(char *data, loff_t offset, size_t size,
 {
 	struct page *ecryptfs_page;
 	char *ecryptfs_page_virt;
-	u64 ecryptfs_file_size = i_size_read(ecryptfs_file->f_dentry->d_inode);
+	loff_t ecryptfs_file_size =
+		i_size_read(ecryptfs_file->f_dentry->d_inode);
 	loff_t data_offset = 0;
 	loff_t pos;
 	int rc = 0;