This patch set addresses file locking, NULL pointer checks, and fsync
issues that Christoph Hellwig pointed out. There are other remaining
issues that Christoph brought up that require more work; we will be
sending another patch set to address those issues in the near future.
Mike
Convert data types in structures in the header to be unsigned if they
are supposed to ever only contain unsigned values.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/ecryptfs_kernel.h | 46 +++++++++++++++++++++--------------------
1 files changed, 23 insertions(+), 23 deletions(-)
704eeca89309cfaed08992f03e100d465b458b56
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 48fd0a1..6164161 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -80,18 +80,18 @@ #define ECRYPTFS_USERSPACE_SHOULD_TRY_TO
#define ECRYPTFS_USERSPACE_SHOULD_TRY_TO_ENCRYPT 0x00000002
#define ECRYPTFS_CONTAINS_DECRYPTED_KEY 0x00000004
#define ECRYPTFS_CONTAINS_ENCRYPTED_KEY 0x00000008
- s32 flags;
- s32 encrypted_key_size;
- s32 decrypted_key_size;
+ u32 flags;
+ u32 encrypted_key_size;
+ u32 decrypted_key_size;
u8 encrypted_key[ECRYPTFS_MAX_ENCRYPTED_KEY_BYTES];
u8 decrypted_key[ECRYPTFS_MAX_KEY_BYTES];
};
struct ecryptfs_password {
- s32 password_bytes;
+ u32 password_bytes;
s32 hash_algo;
- s32 hash_iterations;
- s32 session_key_encryption_key_bytes;
+ u32 hash_iterations;
+ u32 session_key_encryption_key_bytes;
#define ECRYPTFS_PERSISTENT_PASSWORD 0x01
#define ECRYPTFS_SESSION_KEY_ENCRYPTION_KEY_SET 0x02
u32 flags;
@@ -104,15 +104,15 @@ #define ECRYPTFS_SESSION_KEY_ENCRYPTION_
/* May be a password or a private key */
struct ecryptfs_auth_tok {
- uint16_t version; /* 8-bit major and 8-bit minor */
+ u16 version; /* 8-bit major and 8-bit minor */
#define ECRYPTFS_PASSWORD 0x00000001
#define ECRYPTFS_PRIVATE_KEY 0x00000002
#define ECRYPTFS_CONTAINS_SECRET 0x00000004
#define ECRYPTFS_EXPIRED 0x00000008
u32 flags;
uid_t uid;
- s64 creation_time;
- s64 expiration_time;
+ u64 creation_time;
+ u64 expiration_time;
union {
struct ecryptfs_password password;
/* Private key is in future eCryptfs releases */
@@ -143,7 +143,7 @@ struct ecryptfs_page_crypt_context {
struct page *page;
#define ECRYPTFS_PREPARE_COMMIT_MODE 0
#define ECRYPTFS_WRITEPAGE_MODE 1
- int mode;
+ unsigned int mode;
union {
struct file *lower_file;
struct writeback_control *wbc;
@@ -191,23 +191,22 @@ #define ECRYPTFS_ENABLE_HMAC 0x00
#define ECRYPTFS_ENCRYPT_IV_PAGES 0x00000040
#define ECRYPTFS_KEY_VALID 0x00000080
u32 flags;
- int file_version;
- int iv_bytes;
- int num_keysigs;
- int header_extent_size;
- int num_header_extents_at_front; /* Number of header extents
- * at the front of the file */
- int extent_size; /* Data extent size; default is 4096 */
- int key_size_bits;
+ unsigned int file_version;
+ unsigned int iv_bytes;
+ unsigned int num_keysigs;
+ unsigned int header_extent_size;
+ unsigned int num_header_extents_at_front;
+ unsigned int extent_size; /* Data extent size; default is 4096 */
+ unsigned int key_size_bits;
unsigned int extent_shift;
unsigned int extent_mask;
struct crypto_tfm *tfm;
struct crypto_tfm *md5_tfm; /* Crypto context for generating
* the initialization vectors */
- char cipher[ECRYPTFS_MAX_CIPHER_NAME_SIZE];
+ unsigned char cipher[ECRYPTFS_MAX_CIPHER_NAME_SIZE];
unsigned char key[ECRYPTFS_MAX_KEY_BYTES];
unsigned char root_iv[ECRYPTFS_MAX_IV_BYTES];
- char keysigs[ECRYPTFS_MAX_NUM_KEYSIGS][ECRYPTFS_SIG_SIZE_HEX];
+ unsigned char keysigs[ECRYPTFS_MAX_NUM_KEYSIGS][ECRYPTFS_SIG_SIZE_HEX];
struct mutex cs_mutex;
};
@@ -234,8 +233,9 @@ struct ecryptfs_mount_crypt_stat {
/* Pointers to memory we do not own, do not free these */
struct ecryptfs_auth_tok *global_auth_tok;
struct key *global_auth_tok_key;
- char global_default_cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE + 1];
- char global_auth_tok_sig[ECRYPTFS_SIG_SIZE_HEX + 1];
+ unsigned char global_default_cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE
+ + 1];
+ unsigned char global_auth_tok_sig[ECRYPTFS_SIG_SIZE_HEX + 1];
};
/* superblock private data. */
@@ -253,7 +253,7 @@ struct ecryptfs_file_info {
/* auth_tok <=> encrypted_session_key mappings */
struct ecryptfs_auth_tok_list_item {
- char encrypted_session_key[ECRYPTFS_MAX_KEY_BYTES];
+ unsigned char encrypted_session_key[ECRYPTFS_MAX_KEY_BYTES];
struct list_head list;
struct ecryptfs_auth_tok auth_tok;
};
--
1.3.3
uint16_t replaced with more kernel-appropriate u16.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
46e9182ba5ba7c1b503f23b69f3accce416f1b71
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index a0e9f05..9376482 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -141,7 +141,7 @@ static match_table_t tokens = {
*
* Returns zero on good version; non-zero otherwise
*/
-int ecryptfs_verify_version(uint16_t version)
+int ecryptfs_verify_version(u16 version)
{
int rc = 0;
unsigned char major;
--
1.3.3
Clean up a couple of #include's in the header file. Looks like there
may be a few more places where the #include's could be tweaked; we'll
address that in a future patch set.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/ecryptfs_kernel.h | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
41e02f240e66ee9c74871eaf0cadd3581cbb1980
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index b58e515..48fd0a1 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -32,8 +32,7 @@ #endif
#include <keys/user-type.h>
#include <linux/fs.h>
-#include <asm/semaphore.h>
-#include <asm/scatterlist.h>
+#include <linux/scatterlist.h>
/* Version verification for shared data structures w/ userspace */
#ifndef ECRYPTFS_VERSION_MAJOR
--
1.3.3
Rewrite ecryptfs_fsync() to be more concise. Note that i_fop is always
valid.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/file.c | 29 ++++++++---------------------
1 files changed, 8 insertions(+), 21 deletions(-)
fb9079aa454161fdbdc53021b987eec7513fec06
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 3a9cd15..c984ea6 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -322,29 +322,16 @@ static int ecryptfs_release(struct inode
static int
ecryptfs_fsync(struct file *file, struct dentry *dentry, int datasync)
{
+ struct file *lower_file = ecryptfs_file_to_lower(file);
+ struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
+ struct inode *lower_inode = lower_dentry->d_inode;
int rc = -EINVAL;
- struct file *lower_file = NULL;
- struct dentry *lower_dentry;
- if (!file) {
- lower_dentry = ecryptfs_dentry_to_lower(dentry);
- if (lower_dentry->d_inode->i_fop
- && lower_dentry->d_inode->i_fop->fsync) {
- mutex_lock(&lower_dentry->d_inode->i_mutex);
- rc = lower_dentry->d_inode->i_fop->fsync(lower_file,
- lower_dentry,
- datasync);
- mutex_unlock(&lower_dentry->d_inode->i_mutex);
- }
- } else {
- lower_file = ecryptfs_file_to_lower(file);
- lower_dentry = ecryptfs_dentry_to_lower(dentry);
- if (lower_file->f_op && lower_file->f_op->fsync) {
- mutex_lock(&lower_dentry->d_inode->i_mutex);
- rc = lower_file->f_op->fsync(lower_file, lower_dentry,
- datasync);
- mutex_unlock(&lower_dentry->d_inode->i_mutex);
- }
+ if (lower_inode->i_fop->fsync) {
+ mutex_lock(&lower_inode->i_mutex);
+ rc = lower_inode->i_fop->fsync(lower_file, lower_dentry,
+ datasync);
+ mutex_unlock(&lower_inode->i_mutex);
}
return rc;
}
--
1.3.3
Use the kernel BUG_ON() macro rather than the eCryptfs ASSERT(). Note
that this temporarily renders the CONFIG_ECRYPT_DEBUG build option
unused. We certainly plan on using it in the future; for now, is it
okay to leave it in fs/Kconfig, or would you like to remove it?
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/crypto.c | 16 ++++++++--------
fs/ecryptfs/ecryptfs_kernel.h | 17 -----------------
fs/ecryptfs/file.c | 4 ++--
fs/ecryptfs/inode.c | 18 +++++++++---------
fs/ecryptfs/keystore.c | 2 +-
fs/ecryptfs/mmap.c | 8 ++++----
6 files changed, 24 insertions(+), 41 deletions(-)
acfbd08111d5f3d1f0aff5064791f86260899aeb
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 809d9f5..49b7eb3 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -245,9 +245,9 @@ static int encrypt_scatterlist(struct ec
{
int rc = 0;
- ASSERT(crypt_stat && crypt_stat->tfm
- && ECRYPTFS_CHECK_FLAG(crypt_stat->flags,
- ECRYPTFS_STRUCT_INITIALIZED));
+ BUG_ON(!crypt_stat || !crypt_stat->tfm
+ || !ECRYPTFS_CHECK_FLAG(crypt_stat->flags,
+ ECRYPTFS_STRUCT_INITIALIZED));
if (unlikely(ecryptfs_verbosity > 0)) {
ecryptfs_printk(KERN_DEBUG, "Key size [%d]; key:\n",
crypt_stat->key_size_bits / 8);
@@ -467,8 +467,8 @@ #define ECRYPTFS_PAGE_STATE_WRITTEN 3
prior_lower_page_idx = lower_page_idx;
page_state = ECRYPTFS_PAGE_STATE_READ;
}
- ASSERT(page_state == ECRYPTFS_PAGE_STATE_MODIFIED
- || page_state == ECRYPTFS_PAGE_STATE_READ);
+ BUG_ON(!(page_state == ECRYPTFS_PAGE_STATE_MODIFIED
+ || page_state == ECRYPTFS_PAGE_STATE_READ));
rc = ecryptfs_derive_iv(extent_iv, crypt_stat,
(base_extent + extent_offset));
if (rc) {
@@ -505,7 +505,7 @@ #define ECRYPTFS_PAGE_STATE_WRITTEN 3
page_state = ECRYPTFS_PAGE_STATE_MODIFIED;
extent_offset++;
}
- ASSERT(orig_byte_offset == 0);
+ BUG_ON(orig_byte_offset != 0);
rc = ecryptfs_write_out_page(ctx, lower_page, lower_inode, 0,
(lower_byte_offset
+ crypt_stat->extent_size));
@@ -792,8 +792,8 @@ int ecryptfs_compute_root_iv(struct ecry
int rc = 0;
char dst[MD5_DIGEST_SIZE];
- ASSERT(crypt_stat->iv_bytes <= MD5_DIGEST_SIZE);
- ASSERT(crypt_stat->iv_bytes > 0);
+ BUG_ON(crypt_stat->iv_bytes > MD5_DIGEST_SIZE);
+ BUG_ON(crypt_stat->iv_bytes <= 0);
if (!ECRYPTFS_CHECK_FLAG(crypt_stat->flags, ECRYPTFS_KEY_VALID)) {
rc = -EINVAL;
ecryptfs_printk(KERN_WARNING, "Session key not valid; "
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index b58e515..14b3f99 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -26,10 +26,6 @@
#ifndef ECRYPTFS_KERNEL_H
#define ECRYPTFS_KERNEL_H
-#ifdef CONFIG_ECRYPT_DEBUG
-#define OBSERVE_ASSERTS 1
-#endif
-
#include <keys/user-type.h>
#include <linux/fs.h>
#include <asm/semaphore.h>
@@ -259,19 +255,6 @@ struct ecryptfs_auth_tok_list_item {
struct ecryptfs_auth_tok auth_tok;
};
-#ifdef OBSERVE_ASSERTS
-#define ASSERT(EX) \
-do { \
- if (unlikely(!(EX))) { \
- printk(KERN_CRIT "ASSERTION FAILED: %s at %s:%d (%s)\n", #EX, \
- __FILE__, __LINE__, __FUNCTION__); \
- BUG(); \
- } \
-} while (0)
-#else
-#define ASSERT(EX) do { /* nothing */ } while (0)
-#endif /* OBSERVE_ASSERTS */
-
static inline struct ecryptfs_file_info *
ecryptfs_file_to_private(struct file *file)
{
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index cb03aee..ef7d7fa 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -530,7 +530,7 @@ static int ecryptfs_lock(struct file *fi
if (ecryptfs_file_to_private(file))
lower_file = ecryptfs_file_to_lower(file);
- ASSERT(lower_file);
+ BUG_ON(!lower_file);
rc = -EINVAL;
if (!fl)
goto out;
@@ -560,7 +560,7 @@ static ssize_t ecryptfs_sendfile(struct
if (ecryptfs_file_to_private(file))
lower_file = ecryptfs_file_to_lower(file);
- ASSERT(lower_file);
+ BUG_ON(!lower_file);
if (lower_file->f_op && lower_file->f_op->sendfile)
rc = lower_file->f_op->sendfile(lower_file, ppos, count,
actor, target);
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 3505dd7..c24b043 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -61,15 +61,15 @@ void ecryptfs_copy_inode_size(struct ino
void ecryptfs_copy_attr_atime(struct inode *dest, const struct inode *src)
{
- ASSERT(dest != NULL);
- ASSERT(src != NULL);
+ BUG_ON(!dest);
+ BUG_ON(!src);
dest->i_atime = src->i_atime;
}
void ecryptfs_copy_attr_times(struct inode *dest, const struct inode *src)
{
- ASSERT(dest != NULL);
- ASSERT(src != NULL);
+ BUG_ON(!dest);
+ BUG_ON(!src);
dest->i_atime = src->i_atime;
dest->i_mtime = src->i_mtime;
dest->i_ctime = src->i_ctime;
@@ -78,8 +78,8 @@ void ecryptfs_copy_attr_times(struct ino
static void ecryptfs_copy_attr_timesizes(struct inode *dest,
const struct inode *src)
{
- ASSERT(dest != NULL);
- ASSERT(src != NULL);
+ BUG_ON(!dest);
+ BUG_ON(!src);
dest->i_atime = src->i_atime;
dest->i_mtime = src->i_mtime;
dest->i_ctime = src->i_ctime;
@@ -88,8 +88,8 @@ static void ecryptfs_copy_attr_timesizes
void ecryptfs_copy_attr_all(struct inode *dest, const struct inode *src)
{
- ASSERT(dest != NULL);
- ASSERT(src != NULL);
+ BUG_ON(!dest);
+ BUG_ON(!src);
dest->i_mode = src->i_mode;
dest->i_nlink = src->i_nlink;
dest->i_uid = src->i_uid;
@@ -392,7 +392,7 @@ static struct dentry *ecryptfs_lookup(st
lower_dentry->d_name.name);
lower_inode = lower_dentry->d_inode;
ecryptfs_copy_attr_atime(dir, lower_dir_dentry->d_inode);
- ASSERT(atomic_read(&lower_dentry->d_count));
+ BUG_ON(!atomic_read(&lower_dentry->d_count));
ecryptfs_set_dentry_private(dentry,
kmem_cache_alloc(ecryptfs_dentry_info_cache,
SLAB_KERNEL));
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index 19ba826..7c5ac0d 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -506,7 +506,7 @@ static int decrypt_session_key(struct ec
auth_tok->session_key.encrypted_key_size);
src_sg[0].page = virt_to_page(encrypted_session_key);
src_sg[0].offset = 0;
- ASSERT(auth_tok->session_key.encrypted_key_size < PAGE_CACHE_SIZE);
+ BUG_ON(auth_tok->session_key.encrypted_key_size > PAGE_CACHE_SIZE);
src_sg[0].length = auth_tok->session_key.encrypted_key_size;
dst_sg[0].page = virt_to_page(session_key);
dst_sg[0].offset = 0;
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 58f5e12..0c0411a 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -280,7 +280,7 @@ static int ecryptfs_readpage(struct file
int rc = 0;
struct ecryptfs_crypt_stat *crypt_stat;
- ASSERT(file && file->f_dentry && file->f_dentry->d_inode);
+ BUG_ON(!(file && file->f_dentry && file->f_dentry->d_inode));
crypt_stat =
&ecryptfs_inode_to_private(file->f_dentry->d_inode)->crypt_stat;
if (!crypt_stat
@@ -553,7 +553,7 @@ process_new_file(struct ecryptfs_crypt_s
header_pages = ((crypt_stat->header_extent_size
* crypt_stat->num_header_extents_at_front)
/ PAGE_CACHE_SIZE);
- ASSERT(header_pages >= 1);
+ BUG_ON(header_pages < 1);
while (current_header_page < header_pages) {
rc = ecryptfs_grab_and_map_lower_page(&header_page,
&header_virt,
@@ -646,8 +646,8 @@ static int ecryptfs_commit_write(struct
mutex_lock(&lower_inode->i_mutex);
crypt_stat =
&ecryptfs_inode_to_private(file->f_dentry->d_inode)->crypt_stat;
- ASSERT(crypt_stat);
- ASSERT(lower_file);
+ BUG_ON(!crypt_stat);
+ BUG_ON(!lower_file);
if (ECRYPTFS_CHECK_FLAG(crypt_stat->flags, ECRYPTFS_NEW_FILE)) {
ecryptfs_printk(KERN_DEBUG, "ECRYPTFS_NEW_FILE flag set in "
"crypt_stat at memory location [%p]\n", crypt_stat);
--
1.3.3
Sorry these got in there; they are really only useful for the
userspace tools.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/ecryptfs_kernel.h | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
28ea0fb87bb7dafb8dbf327f86359b9e80c40bb5
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e8b8691..8e35dbd 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -31,16 +31,9 @@ #include <linux/fs.h>
#include <linux/scatterlist.h>
/* Version verification for shared data structures w/ userspace */
-#ifndef ECRYPTFS_VERSION_MAJOR
#define ECRYPTFS_VERSION_MAJOR 0x00
-#endif
-#ifndef ECRYPTFS_VERSION_MINOR
#define ECRYPTFS_VERSION_MINOR 0x01
-#endif
-
-#ifndef ECRYPTFS_SUPPORTED_FILE_VERSION
#define ECRYPTFS_SUPPORTED_FILE_VERSION 0x01
-#endif
#define ECRYPTFS_MAX_PASSWORD_LENGTH 64
#define ECRYPTFS_MAX_PASSPHRASE_BYTES ECRYPTFS_MAX_PASSWORD_LENGTH
@@ -168,9 +161,7 @@ #define ECRYPTFS_DEFAULT_KEY_BYTES 16
#define ECRYPTFS_DEFAULT_CHAINING_MODE CRYPTO_TFM_MODE_CBC
#define ECRYPTFS_TAG_3_PACKET_TYPE 0x8C
#define ECRYPTFS_TAG_11_PACKET_TYPE 0xED
-#ifndef MD5_DIGEST_SIZE
#define MD5_DIGEST_SIZE 16
-#endif
/**
* This is the primary struct associated with each encrypted file.
--
1.3.3
Remove ``NULL =='' syntax; easier on the eyes.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/file.c | 2 +-
fs/ecryptfs/main.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
c6a4cc09586f339bd97330efcd388c92100a6780
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index ef7d7fa..0284e16 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -371,7 +371,7 @@ ecryptfs_fsync(struct file *file, struct
struct file *lower_file = NULL;
struct dentry *lower_dentry;
- if (NULL == file) {
+ if (!file) {
lower_dentry = ecryptfs_dentry_to_lower(dentry);
if (lower_dentry->d_inode->i_fop
&& lower_dentry->d_inode->i_fop->fsync) {
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 9376482..5cbc948 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -98,7 +98,7 @@ int ecryptfs_interpose(struct dentry *lo
rc = -ENOMEM;
goto out;
}
- if (NULL == ecryptfs_inode_to_lower(inode))
+ if (!ecryptfs_inode_to_lower(inode))
ecryptfs_set_inode_lower(inode, igrab(lower_inode));
if (S_ISLNK(lower_inode->i_mode))
inode->i_op = &ecryptfs_symlink_iops;
--
1.3.3
Christoph Hellwig pointed out that the inode is not live at this point
in ecryptfs_open(), and so this entire function is pointless.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/file.c | 47 +----------------------------------------------
1 files changed, 1 insertions(+), 46 deletions(-)
35776954e1116224272c4e191aed9a85b63914f7
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 0284e16..b8fcd0a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -189,49 +189,6 @@ retry:
return rc;
}
-/**
- * read_inode_size_from_header
- * @lower_file: The lower file struct
- * @lower_inode: The lower inode
- * @inode: The ecryptfs inode
- *
- * Returns zero on success; non-zero otherwise
- */
-static int
-read_inode_size_from_header(struct file *lower_file,
- struct inode *lower_inode, struct inode *inode)
-{
- int rc;
- struct page *header_page;
- unsigned char *header_virt;
- u64 data_size;
-
- header_page = grab_cache_page(lower_inode->i_mapping, 0);
- if (!header_page) {
- rc = -EINVAL;
- ecryptfs_printk(KERN_ERR, "grab_cache_page for header page "
- "failed\n");
- goto out;
- }
- header_virt = kmap(header_page);
- rc = lower_inode->i_mapping->a_ops->readpage(lower_file, header_page);
- if (rc) {
- ecryptfs_printk(KERN_ERR, "Error reading header page\n");
- goto out_unmap;
- }
- memcpy(&data_size, header_virt, sizeof(data_size));
- data_size = be64_to_cpu(data_size);
- i_size_write(inode, (loff_t)data_size);
- ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16x] "
- "size: [0x%.16x]\n", inode, inode->i_ino,
- i_size_read(inode));
-out_unmap:
- kunmap(header_page);
- page_cache_release(header_page);
-out:
- return rc;
-}
-
struct kmem_cache *ecryptfs_file_info_cache;
/**
@@ -320,9 +277,7 @@ static int ecryptfs_open(struct inode *i
* going to default to -EIO. */
rc = -EIO;
goto out_puts;
- } else
- read_inode_size_from_header(lower_file, lower_inode,
- inode);
+ }
}
ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16x] "
"size: [0x%.16x]\n", inode, inode->i_ino,
--
1.3.3
Use exported posix_lock_file_wait() rather than re-implement it. Note
that ecryptfs_getlk() and ecryptfs_setlk() still need work; we need to
figure out how to best integrate with the existing fcntl_setlk() and
fcntl_getlk() functions in fs/locks.c.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/file.c | 71 +++++++++++++---------------------------------------
1 files changed, 18 insertions(+), 53 deletions(-)
494162519c1eee8de78405ac35302f488a6b8ef0
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index c984ea6..c4ea9f0 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -336,40 +336,12 @@ ecryptfs_fsync(struct file *file, struct
return rc;
}
-static void locks_delete_block(struct file_lock *waiter)
-{
- lock_kernel();
- list_del_init(&waiter->fl_block);
- list_del_init(&waiter->fl_link);
- waiter->fl_next = NULL;
- unlock_kernel();
-}
-
-static int ecryptfs_posix_lock(struct file *file, struct file_lock *fl, int cmd)
-{
- int rc;
-
-lock_file:
- rc = posix_lock_file(file, fl);
- if ((rc != -EAGAIN) || (cmd == F_SETLK))
- goto out;
- rc = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
- if (!rc)
- goto lock_file;
- locks_delete_block(fl);
-out:
- return rc;
-}
-
static int ecryptfs_setlk(struct file *file, int cmd, struct file_lock *fl)
{
- int rc = -EINVAL;
- struct inode *inode, *lower_inode;
- struct file *lower_file = NULL;
+ struct file *lower_file = ecryptfs_file_to_lower(file);
+ struct inode *lower_inode = lower_file->f_dentry->d_inode;
+ int rc;
- lower_file = ecryptfs_file_to_lower(file);
- inode = file->f_dentry->d_inode;
- lower_inode = lower_file->f_dentry->d_inode;
/* Don't allow mandatory locks on files that may be memory mapped
* and shared. */
if (IS_MANDLOCK(lower_inode) &&
@@ -378,7 +350,7 @@ static int ecryptfs_setlk(struct file *f
rc = -EAGAIN;
goto out;
}
- if (cmd == F_SETLKW)
+ if (IS_SETLKW(cmd))
fl->fl_flags |= FL_SLEEP;
rc = -EBADF;
switch (fl->fl_type) {
@@ -406,29 +378,29 @@ static int ecryptfs_setlk(struct file *f
goto out;
goto upper_lock;
}
- rc = ecryptfs_posix_lock(lower_file, fl, cmd);
+ rc = posix_lock_file_wait(lower_file, fl);
if (rc)
goto out;
upper_lock:
fl->fl_file = file;
- rc = ecryptfs_posix_lock(file, fl, cmd);
+ rc = posix_lock_file_wait(lower_file, fl);
if (rc) {
fl->fl_type = F_UNLCK;
fl->fl_file = lower_file;
- ecryptfs_posix_lock(lower_file, fl, cmd);
+ rc = posix_lock_file_wait(lower_file, fl);
}
out:
return rc;
}
-static int ecryptfs_getlk(struct file *file, struct file_lock *fl)
+static int ecryptfs_getlk(struct file *file, int cmd, struct file_lock *fl)
{
struct file_lock cfl;
struct file_lock *tempfl = NULL;
int rc = 0;
if (file->f_op && file->f_op->lock) {
- rc = file->f_op->lock(file, F_GETLK, fl);
+ rc = file->f_op->lock(file, cmd, fl);
if (rc < 0)
goto out;
} else
@@ -454,27 +426,20 @@ static int ecryptfs_fasync(int fd, struc
static int ecryptfs_lock(struct file *file, int cmd, struct file_lock *fl)
{
- int rc = 0;
- struct file *lower_file = NULL;
+ struct file *lower_file = ecryptfs_file_to_lower(file);
+ int rc = -EINVAL;
- lower_file = ecryptfs_file_to_lower(file);
- rc = -EINVAL;
if (!fl)
goto out;
- fl->fl_file = lower_file;
- switch (cmd) {
- case F_GETLK:
- rc = ecryptfs_getlk(lower_file, fl);
- break;
- case F_SETLK:
- case F_SETLKW:
+ if (IS_GETLK(cmd)) {
+ fl->fl_file = lower_file;
+ rc = ecryptfs_getlk(lower_file, cmd, fl);
+ fl->fl_file = file;
+ } else if (IS_SETLK(cmd) || IS_SETLKW(cmd)) {
fl->fl_file = file;
rc = ecryptfs_setlk(file, cmd, fl);
- break;
- default:
- rc = -EINVAL;
- }
- fl->fl_file = file;
+ } else
+ fl->fl_file = file;
out:
return rc;
}
--
1.3.3
Remove unnecessary checks for NULL for values that should never be
NULL. A couple of minor comment/spacing fixes.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/ecryptfs/dentry.c | 4 ----
fs/ecryptfs/file.c | 24 ++++--------------------
fs/ecryptfs/inode.c | 18 ------------------
fs/ecryptfs/mmap.c | 7 -------
fs/ecryptfs/super.c | 3 ---
5 files changed, 4 insertions(+), 52 deletions(-)
080439fe5ee41e1cff6f947722c7be027c295583
diff --git a/fs/ecryptfs/dentry.c b/fs/ecryptfs/dentry.c
index 12362c0..7b1018a 100644
--- a/fs/ecryptfs/dentry.c
+++ b/fs/ecryptfs/dentry.c
@@ -47,10 +47,6 @@ static int ecryptfs_d_revalidate(struct
struct vfsmount *saved_vfsmount;
lower_dentry = ecryptfs_dentry_to_lower(dentry);
- if (!lower_dentry) {
- err = 0;
- goto out;
- }
if (!lower_dentry->d_op || !lower_dentry->d_op->d_revalidate)
goto out;
saved_dentry = nd->dentry;
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index b8fcd0a..3a9cd15 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -337,12 +337,6 @@ ecryptfs_fsync(struct file *file, struct
mutex_unlock(&lower_dentry->d_inode->i_mutex);
}
} else {
- if (!ecryptfs_file_to_private(file)) {
- rc = -EINVAL;
- ecryptfs_printk(KERN_ERR, "ecryptfs_file_to_private"
- "(file=[%p]) is NULL\n", file);
- goto out;
- }
lower_file = ecryptfs_file_to_lower(file);
lower_dentry = ecryptfs_dentry_to_lower(dentry);
if (lower_file->f_op && lower_file->f_op->fsync) {
@@ -352,7 +346,6 @@ ecryptfs_fsync(struct file *file, struct
mutex_unlock(&lower_dentry->d_inode->i_mutex);
}
}
-out:
return rc;
}
@@ -466,15 +459,9 @@ static int ecryptfs_fasync(int fd, struc
int rc = 0;
struct file *lower_file = NULL;
- if (NULL != ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
- else {
- rc = -EINVAL;
- goto out;
- }
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->fasync)
rc = lower_file->f_op->fasync(fd, lower_file, flag);
-out:
return rc;
}
@@ -483,9 +470,7 @@ static int ecryptfs_lock(struct file *fi
int rc = 0;
struct file *lower_file = NULL;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
- BUG_ON(!lower_file);
+ lower_file = ecryptfs_file_to_lower(file);
rc = -EINVAL;
if (!fl)
goto out;
@@ -513,9 +498,7 @@ static ssize_t ecryptfs_sendfile(struct
struct file *lower_file = NULL;
int rc = -EINVAL;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
- BUG_ON(!lower_file);
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->sendfile)
rc = lower_file->f_op->sendfile(lower_file, ppos, count,
actor, target);
@@ -561,6 +544,7 @@ ecryptfs_ioctl(struct inode *inode, stru
{
int rc = 0;
struct file *lower_file = NULL;
+
if (ecryptfs_file_to_private(file))
lower_file = ecryptfs_file_to_lower(file);
if (lower_file && lower_file->f_op && lower_file->f_op->ioctl)
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index c24b043..47e4202 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -159,12 +159,6 @@ ecryptfs_do_create(struct inode *directo
struct dentry *lower_dir_dentry;
lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry);
- if (IS_ERR(lower_dentry)) {
- ecryptfs_printk(KERN_ERR, "ecryptfs dentry doesn't know"
- "about its lower counterpart\n");
- rc = PTR_ERR(lower_dentry);
- goto out;
- }
lower_dir_dentry = lock_parent(lower_dentry);
if (unlikely(IS_ERR(lower_dir_dentry))) {
ecryptfs_printk(KERN_ERR, "Error locking directory of "
@@ -252,12 +246,6 @@ static int ecryptfs_initialize_file(stru
struct vfsmount *lower_mnt;
lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry);
- if (IS_ERR(lower_dentry)) {
- ecryptfs_printk(KERN_ERR, "ecryptfs dentry doesn't know"
- "about its lower counterpart\n");
- rc = PTR_ERR(lower_dentry);
- goto out;
- }
ecryptfs_printk(KERN_DEBUG, "lower_dentry->d_name.name = [%s]\n",
lower_dentry->d_name.name);
inode = ecryptfs_dentry->d_inode;
@@ -831,12 +819,6 @@ int ecryptfs_truncate(struct dentry *den
if (unlikely((new_length == i_size)))
goto out;
crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
- if (unlikely(!crypt_stat)) {
- ecryptfs_printk(KERN_ERR, "NULL crypt_stat on dentry with "
- "d_name.name = [%s]\n", dentry->d_name.name);
- rc = -EINVAL;
- goto out;
- }
/* Set up a fake ecryptfs file, this is used to interface with
* the file in the underlying filesystem so that the
* truncation has an effect there as well. */
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index 0c0411a..e9cff02 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -219,11 +219,6 @@ int ecryptfs_do_readpage(struct file *fi
const struct address_space_operations *lower_a_ops;
dentry = file->f_dentry;
- if (!ecryptfs_file_to_private(file)) {
- rc = -ENOENT;
- ecryptfs_printk(KERN_ERR, "No lower file info\n");
- goto out;
- }
lower_file = ecryptfs_file_to_lower(file);
lower_dentry = ecryptfs_dentry_to_lower(dentry);
inode = dentry->d_inode;
@@ -646,8 +641,6 @@ static int ecryptfs_commit_write(struct
mutex_lock(&lower_inode->i_mutex);
crypt_stat =
&ecryptfs_inode_to_private(file->f_dentry->d_inode)->crypt_stat;
- BUG_ON(!crypt_stat);
- BUG_ON(!lower_file);
if (ECRYPTFS_CHECK_FLAG(crypt_stat->flags, ECRYPTFS_NEW_FILE)) {
ecryptfs_printk(KERN_DEBUG, "ECRYPTFS_NEW_FILE flag set in "
"crypt_stat at memory location [%p]\n", crypt_stat);
diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c
index 1d344e0..94c50f1 100644
--- a/fs/ecryptfs/super.c
+++ b/fs/ecryptfs/super.c
@@ -37,9 +37,6 @@ struct kmem_cache *ecryptfs_inode_info_c
*
* Called to bring an inode into existence.
*
- * Note that setting the self referencing pointer doesn't work here:
- * i.e. ECRYPTFS_INODE_TO_PRIVATE_SM(inode) = ei;
- *
* Only handle allocation, setting up structures should be done in
* ecryptfs_read_inode. This is because the kernel, between now and
* then, will 0 out the private data pointer.
--
1.3.3
On Fri, May 26, 2006 at 09:21:48AM -0500, Mike Halcrow wrote:
> Use the kernel BUG_ON() macro rather than the eCryptfs ASSERT(). Note
> that this temporarily renders the CONFIG_ECRYPT_DEBUG build option
> unused. We certainly plan on using it in the future; for now, is it
> okay to leave it in fs/Kconfig, or would you like to remove it?
>...
Remove it as long as it deos nothing - re-adding it when it's actually
used will be trivial.
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -506,7 +506,7 @@ static int decrypt_session_key(struct ec
> auth_tok->session_key.encrypted_key_size);
> src_sg[0].page = virt_to_page(encrypted_session_key);
> src_sg[0].offset = 0;
> - ASSERT(auth_tok->session_key.encrypted_key_size < PAGE_CACHE_SIZE);
> + BUG_ON(auth_tok->session_key.encrypted_key_size > PAGE_CACHE_SIZE);
>...
What's with (auth_tok->session_key.encrypted_key_size == PAGE_CACHE_SIZE) ?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Fri, May 26, 2006 at 05:24:01PM +0200, Adrian Bunk wrote:
> On Fri, May 26, 2006 at 09:21:48AM -0500, Mike Halcrow wrote:
> > - ASSERT(auth_tok->session_key.encrypted_key_size < PAGE_CACHE_SIZE);
> > + BUG_ON(auth_tok->session_key.encrypted_key_size > PAGE_CACHE_SIZE);
> >...
>
> What's with (auth_tok->session_key.encrypted_key_size ==
> PAGE_CACHE_SIZE) ?
That should not be a problem.
Mike
On Fri, May 26, 2006 at 11:14:54AM -0500, Michael Halcrow wrote:
> On Fri, May 26, 2006 at 05:24:01PM +0200, Adrian Bunk wrote:
> > On Fri, May 26, 2006 at 09:21:48AM -0500, Mike Halcrow wrote:
> > > - ASSERT(auth_tok->session_key.encrypted_key_size < PAGE_CACHE_SIZE);
> > > + BUG_ON(auth_tok->session_key.encrypted_key_size > PAGE_CACHE_SIZE);
> > >...
> >
> > What's with (auth_tok->session_key.encrypted_key_size ==
> > PAGE_CACHE_SIZE) ?
>
> That should not be a problem.
IOW, the ASSERT was wrong?
My point is simply that this is not a semantically equivalent
transformation, so if this fixes a case where the ASSERT() might have
mistakenly triggered it should be noted in the changelog.
> Mike
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Fri, May 26, 2006 at 05:24:01PM +0200, Adrian Bunk wrote:
> On Fri, May 26, 2006 at 09:21:48AM -0500, Mike Halcrow wrote:
> > Use the kernel BUG_ON() macro rather than the eCryptfs
> > ASSERT(). Note that this temporarily renders the
> > CONFIG_ECRYPT_DEBUG build option unused. We certainly plan on
> > using it in the future; for now, is it okay to leave it in
> > fs/Kconfig, or would you like to remove it?
>
> Remove it as long as it deos nothing - re-adding it when it's
> actually used will be trivial.
This patch removes ECRYPT_DEBUG from fs/Kconfig; apply after the patch
1 from this 10-part patch set.
Signed-off-by: Michael Halcrow <[email protected]>
---
fs/Kconfig | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
15ef83ff75ab0b601a2648175c7f60af16ddb31c
diff --git a/fs/Kconfig b/fs/Kconfig
index 67d5568..b68e46e 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -955,14 +955,6 @@ config ECRYPT_FS
To compile this file system support as a module, choose M here: the
module will be called ecryptfs.
-config ECRYPT_DEBUG
- bool "Enable eCryptfs debug mode"
- depends on ECRYPT_FS
- help
- Turn on debugging code in eCryptfs.
-
- If unsure, say N.
-
config HFS_FS
tristate "Apple Macintosh file system support (EXPERIMENTAL)"
depends on EXPERIMENTAL