Tyler/Dustin,
We came across several related bugs while trying to use the -o
ecryptfs_xattr option while mounting the ecryptfs (in 2.6.31.latest and
2.6.32) which saves its metadata information in extended attributes of the
lower file system. These bugs discovered and fixed by Saumitra Bhanage
while using ecryptfs for a graduate class project. We've reviewed the code
and we're reasonably certain of our fixes below; it'll probably make sense
for someone else to give this patch a careful review.
The first problem was running out of memory, and was reproduced as follows:
# mount -t ecryptfs -o ecryptfs_xattr /root/ecryptfs_base/ /mnt/test_mount/
...
Attempting to mount with the following options:
ecryptfs_xattr_metadata
ecryptfs_key_bytes=16
ecryptfs_cipher=aes
ecryptfs_sig=633937dbcf1fef34
Mounted eCryptfs
# touch /mnt/test_mount/file1
touch: cannot touch `/mnt/test_mount/file1': Cannot allocate memory
Problem: ecryptfs_set_default_sizes() hard-codes the header size to 0 if the
xattr option is enabled. That size is used to decide how much memory to
allocate to hold the header. When you set the num_header_bytes_at_front to
zero, later on ecryptfs_write_metadata will call get_order(0), and
get_order(0) returns the value 20 (if you follow the math there). Next, it
calls ecryptfs_get_zeroed_pages(GFP_KERNEL, 20), which tries (and fails with
ENOMEM) to allocate 2^20 pages.
Solution: set the header size to ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE. This
is the fix to crypto.c:ecryptfs_set_default_sizes.
Fixing the above revealed a new problem. Now, if you fix the header size to
ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE, then now the code in
ecryptfs_lower_offset_for_extent was wrong: it used to assume that the
header size was the default 8k. Now, if we use xattrs, then the
lower_offset computed should exclude the current header size. That is the
patch to crypto.c:ecryptfs_lower_offset_for_extent.
Finally, we've discovered one more place where the header size was
incorrectly computed. When truncating a file, ecryptfs will try to compute
the actual lower truncation point by adding the header size. The code
always assumed that there was a physical in-file header of size
num_header_bytes_at_front. But when using the xattr option, this needs to
be set to zero. This is the patch to fix
inode.c:upper_size_to_lower_size().
Cheers,
Erez.
------------------------------------------------------------------------------
ecryptfs: properly compute header sizes/offsets when storing headers in xattrs
Ecryptfs can store its meta-data in a header which can be either an 8k chunk
at the front of the actual ciphertext file, or stored in extended
attributes. When using this ecryptfs_xattr option, the header sizes and
file offsets are computed incorrectly, resulting in ENOMEM errors, wasted
header spaces, or truncating a file at the wrong offset.
Signed-off-by: Saumitra Bhanage <sbhanage@xxxxxxxxxxxxx>
Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index fbb6e5e..9c26ce7 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -381,8 +381,12 @@ out:
static void ecryptfs_lower_offset_for_extent(loff_t *offset, loff_t extent_num,
struct ecryptfs_crypt_stat *crypt_stat)
{
- (*offset) = (crypt_stat->num_header_bytes_at_front
- + (crypt_stat->extent_size * extent_num));
+ /* if using xattr, exclude no. of header bytes from start offset */
+ if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
+ (*offset) = (crypt_stat->extent_size * extent_num);
+ else
+ (*offset) = (crypt_stat->num_header_bytes_at_front
+ + (crypt_stat->extent_size * extent_num));
}
/**
@@ -833,8 +837,10 @@ void ecryptfs_set_default_sizes(struct ecryptfs_crypt_stat *crypt_stat)
crypt_stat->extent_size = ECRYPTFS_DEFAULT_EXTENT_SIZE;
set_extent_mask_and_shift(crypt_stat);
crypt_stat->iv_bytes = ECRYPTFS_DEFAULT_IV_BYTES;
+ /* metadata is kept in xattr, maintain its size in crypt_stat */
if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
- crypt_stat->num_header_bytes_at_front = 0;
+ crypt_stat->num_header_bytes_at_front =
+ ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE;
else {
if (PAGE_CACHE_SIZE <= ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE)
crypt_stat->num_header_bytes_at_front =
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 2dc0bdb..91a5dba 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -767,7 +767,11 @@ upper_size_to_lower_size(struct ecryptfs_crypt_stat *crypt_stat,
{
loff_t lower_size;
- lower_size = crypt_stat->num_header_bytes_at_front;
+ /* if xattr, then lower file size starts at 0 (no in-file header) */
+ if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
+ lower_size = 0;
+ else
+ lower_size = crypt_stat->num_header_bytes_at_front;
if (upper_size != 0) {
loff_t num_extents;
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
