[PATCH 0/7] eCryptfs: Trivial updates

[PATCH 0/7] eCryptfs: Trivial updates

[Michael Halcrow]

This set of patches make trivial updates to eCryptfs to clean up some
of the code.

[PATCH 1/7] eCryptfs: Remove unnecessary BUG_ON

[Michael Halcrow]

Andrew Morton wrote:
> > +     mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
> > +     BUG_ON(mount_crypt_stat->num_global_auth_toks == 0);
> > +     mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex);
>
> That's odd-looking.  If it was a bug for num_global_auth_toks to be
> zero, and if that mutex protects num_global_auth_toks then as soon
> as the lock gets dropped, another thread can make
> num_global_auth_toks zero, hence the bug is present.  Perhaps?

That was serving as an internal sanity check that should not have made
it into the final patch set in the first place. This patch removes it.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/crypto.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 129ed13..14cc1f5 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1029,9 +1029,6 @@ int ecryptfs_new_file_context(struct dentry *ecryptfs_dentry)
 	int rc = 0;
 
 	ecryptfs_set_default_crypt_stat_vals(crypt_stat, mount_crypt_stat);
-	mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
-	BUG_ON(mount_crypt_stat->num_global_auth_toks == 0);
-	mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex);
 	crypt_stat->flags |= ECRYPTFS_ENCRYPTED;
 	crypt_stat->flags |= ECRYPTFS_KEY_VALID;
 	ecryptfs_copy_mount_wide_flags_to_inode_flags(crypt_stat,
-- 
1.5.1.6

[PATCH 2/7] eCryptfs: Collapse flag set into one statement

[Michael Halcrow]

Andrew Morton wrote:
> > +     crypt_stat->flags |= ECRYPTFS_ENCRYPTED;
> > +     crypt_stat->flags |= ECRYPTFS_KEY_VALID;
>
> Maybe the compiler can optimise those two statements, but we'd
> normally provide it with some manual help.

This patch provides the compiler with some manual help for
optimizing the setting of some flags.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/crypto.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 14cc1f5..76bba73 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1029,8 +1029,7 @@ int ecryptfs_new_file_context(struct dentry *ecryptfs_dentry)
 	int rc = 0;
 
 	ecryptfs_set_default_crypt_stat_vals(crypt_stat, mount_crypt_stat);
-	crypt_stat->flags |= ECRYPTFS_ENCRYPTED;
-	crypt_stat->flags |= ECRYPTFS_KEY_VALID;
+	crypt_stat->flags |= (ECRYPTFS_ENCRYPTED | ECRYPTFS_KEY_VALID);
 	ecryptfs_copy_mount_wide_flags_to_inode_flags(crypt_stat,
 						      mount_crypt_stat);
 	rc = ecryptfs_copy_mount_wide_sigs_to_inode_sigs(crypt_stat,
-- 
1.5.1.6

[PATCH 3/7] eCryptfs: Grammatical fix (destruct to destroy)

[Michael Halcrow]

Andrew Morton wrote:
> > +int ecryptfs_destruct_crypto(void)
>
> ecryptfs_destroy_crypto would be more grammatically correct ;)

Grammatical fix for some function names.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/crypto.c          |    8 ++++----
 fs/ecryptfs/ecryptfs_kernel.h |    6 +++---
 fs/ecryptfs/main.c            |    4 ++--
 fs/ecryptfs/super.c           |    4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 76bba73..6051dbf 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -213,12 +213,12 @@ ecryptfs_init_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat)
 }
 
 /**
- * ecryptfs_destruct_crypt_stat
+ * ecryptfs_destroy_crypt_stat
  * @crypt_stat: Pointer to the crypt_stat struct to initialize.
  *
  * Releases all memory associated with a crypt_stat struct.
  */
-void ecryptfs_destruct_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat)
+void ecryptfs_destroy_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat)
 {
 	struct ecryptfs_key_sig *key_sig, *key_sig_tmp;
 
@@ -236,7 +236,7 @@ void ecryptfs_destruct_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat)
 	memset(crypt_stat, 0, sizeof(struct ecryptfs_crypt_stat));
 }
 
-void ecryptfs_destruct_mount_crypt_stat(
+void ecryptfs_destroy_mount_crypt_stat(
 	struct ecryptfs_mount_crypt_stat *mount_crypt_stat)
 {
 	struct ecryptfs_global_auth_tok *auth_tok, *auth_tok_tmp;
@@ -1880,7 +1880,7 @@ int ecryptfs_init_crypto(void)
 	return 0;
 }
 
-int ecryptfs_destruct_crypto(void)
+int ecryptfs_destroy_crypto(void)
 {
 	struct ecryptfs_key_tfm *key_tfm, *key_tfm_tmp;
 
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 6ddab6c..69f6a22 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -516,8 +516,8 @@ int virt_to_scatterlist(const void *addr, int size, struct scatterlist *sg,
 int ecryptfs_compute_root_iv(struct ecryptfs_crypt_stat *crypt_stat);
 void ecryptfs_rotate_iv(unsigned char *iv);
 void ecryptfs_init_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat);
-void ecryptfs_destruct_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat);
-void ecryptfs_destruct_mount_crypt_stat(
+void ecryptfs_destroy_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat);
+void ecryptfs_destroy_mount_crypt_stat(
 	struct ecryptfs_mount_crypt_stat *mount_crypt_stat);
 int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat);
 int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
@@ -620,7 +620,7 @@ int
 ecryptfs_add_new_key_tfm(struct ecryptfs_key_tfm **key_tfm, char *cipher_name,
 			 size_t key_size);
 int ecryptfs_init_crypto(void);
-int ecryptfs_destruct_crypto(void);
+int ecryptfs_destroy_crypto(void);
 int ecryptfs_get_tfm_and_mutex_for_cipher_name(struct crypto_blkcipher **tfm,
 					       struct mutex **tfm_mutex,
 					       char *cipher_name);
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index dd9d7b5..d1b9a70 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -846,8 +846,8 @@ static void __exit ecryptfs_exit(void)
 {
 	int rc;
 
-	if ((rc = ecryptfs_destruct_crypto())) {
-		printk(KERN_ERR "Failure whilst attempting to destruct crypto; "
+	if ((rc = ecryptfs_destroy_crypto())) {
+		printk(KERN_ERR "Failure whilst attempting to destroy crypto; "
 		       "rc = [%d]\n", rc);
 	}
 	sysfs_remove_file(&ecryptfs_subsys.kobj,
diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c
index 7b3f0cc..18d77f8 100644
--- a/fs/ecryptfs/super.c
+++ b/fs/ecryptfs/super.c
@@ -73,7 +73,7 @@ static void ecryptfs_destroy_inode(struct inode *inode)
 	struct ecryptfs_inode_info *inode_info;
 
 	inode_info = ecryptfs_inode_to_private(inode);
-	ecryptfs_destruct_crypt_stat(&inode_info->crypt_stat);
+	ecryptfs_destroy_crypt_stat(&inode_info->crypt_stat);
 	kmem_cache_free(ecryptfs_inode_info_cache, inode_info);
 }
 
@@ -104,7 +104,7 @@ static void ecryptfs_put_super(struct super_block *sb)
 {
 	struct ecryptfs_sb_info *sb_info = ecryptfs_superblock_to_private(sb);
 
-	ecryptfs_destruct_mount_crypt_stat(&sb_info->mount_crypt_stat);
+	ecryptfs_destroy_mount_crypt_stat(&sb_info->mount_crypt_stat);
 	kmem_cache_free(ecryptfs_sb_info_cache, sb_info);
 	ecryptfs_set_superblock_private(sb, NULL);
 }
-- 
1.5.1.6

[PATCH 4/7] eCryptfs: Comments for some structs

[Michael Halcrow]

Andrew Morton wrote:
> > +struct ecryptfs_global_auth_tok {
> > +#define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
> > +     u32 flags;
> > +     struct list_head mount_crypt_stat_list;
> > +     struct key *global_auth_tok_key;
> > +     struct ecryptfs_auth_tok *global_auth_tok;
> > +     unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1];
> > +};
> > +
> > +struct ecryptfs_key_tfm {
> > +     struct crypto_blkcipher *key_tfm;
> > +     size_t key_size;
> > +     struct mutex key_tfm_mutex;
> > +     struct list_head key_tfm_list;
> > +     unsigned char cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE + 1];
> > +};
>
> Please consider commenting your struct fields carefully: it's a
> great way to help other to understand your code.

Add some comments to the ecryptfs_global_auth_tok and ecryptfs_key_tfm
structs to make their functions more easily ascertained.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/ecryptfs_kernel.h |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 69f6a22..e09e1fb 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -273,21 +273,36 @@ struct ecryptfs_dentry_info {
 	struct ecryptfs_crypt_stat *crypt_stat;
 };
 
+/**
+ * ecryptfs_global_auth_tok structs refer to authentication token keys
+ * in the user keyring that apply to newly created files. A list of
+ * these objects hangs off of the mount_crypt_stat struct for any
+ * given eCryptfs mount. This struct maintains a reference to both the
+ * key contents and the key itself so that the key can be put on
+ * unmount.
+ */
 struct ecryptfs_global_auth_tok {
 #define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
 	u32 flags;
-	struct list_head mount_crypt_stat_list;
-	struct key *global_auth_tok_key;
-	struct ecryptfs_auth_tok *global_auth_tok;
-	unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1];
+	struct list_head mount_crypt_stat_list; /* Default auth_tok list for
+						 * the mount_crypt_stat */
+	struct key *global_auth_tok_key; /* The key from the user's keyring for
+					  * the sig */
+	struct ecryptfs_auth_tok *global_auth_tok; /* The key contents */
+	unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1]; /* The key identifier */
 };
 
+/**
+ * Typically, eCryptfs will use the same ciphers repeatedly throughout
+ * the course of its operations. In order to avoid unnecessarily
+ * destroying and initializing the same cipher repeatedly, eCryptfs
+ * keeps a list of crypto API contexts around to use when needed.
+ */
 struct ecryptfs_key_tfm {
 	struct crypto_blkcipher *key_tfm;
 	size_t key_size;
 	struct mutex key_tfm_mutex;
-	struct list_head key_tfm_list;
+	struct list_head key_tfm_list; /* The module's tfm list */
 	unsigned char cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE + 1];
 };
 
-- 
1.5.1.6

[PATCH 5/7] eCryptfs: kerneldoc fixes for crypto.c and keystore.c

[Michael Halcrow]

Andrew Morton wrote:
From: mhalcrow@us.ibm.com <mhalcrow@halcrow.austin.ibm.com>
> > +/**
> > + * decrypt_passphrase_encrypted_session_key - Decrypt the session key
> > + * with the given auth_tok.
> >   *
> >   * Returns Zero on success; non-zero error otherwise.
> >   */
>
> That comment purports to be a kerneldoc-style comment.  But
>
> - kerneldoc doesn't support multiple lines on the introductory line
>   which identifies the name of the function (alas).  So you'll need to
>   overflow 80 cols here.
>
> - the function args weren't documented
>
> But the return value is!  People regularly forget to do that.  And
> they frequently forget to document the locking prerequisites and the
> permissible calling contexts (process/might_sleep/hardirq, etc)
>
> (please check all ecryptfs kerneldoc for this stuff sometime)

This patch cleans up some of the existing comments and makes a couple
of line break tweaks. There is more work to do to bring eCryptfs into
full kerneldoc-compliance.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/crypto.c   |   69 ++++++++++++++++++++++++++++++++++++------------
 fs/ecryptfs/keystore.c |   48 ++++++++++++++++++--------------
 2 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 6051dbf..7aa2f48 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -722,6 +722,11 @@ out:
 
 /**
  * decrypt_scatterlist
+ * @crypt_stat: Cryptographic context
+ * @dest_sg: The destination scatterlist to decrypt into
+ * @src_sg: The source scatterlist to decrypt from
+ * @size: The number of bytes to decrypt
+ * @iv: The initialization vector to use for the decryption
  *
  * Returns the number of bytes decrypted; negative value on error
  */
@@ -763,6 +768,13 @@ out:
 
 /**
  * ecryptfs_encrypt_page_offset
+ * @crypt_stat: The cryptographic context
+ * @dst_page: The page to encrypt into
+ * @dst_offset: The offset in the page to encrypt into
+ * @src_page: The page to encrypt from
+ * @src_offset: The offset in the page to encrypt from
+ * @size: The number of bytes to encrypt
+ * @iv: The initialization vector to use for the encryption
  *
  * Returns the number of bytes encrypted
  */
@@ -785,6 +797,13 @@ ecryptfs_encrypt_page_offset(struct ecryptfs_crypt_stat *crypt_stat,
 
 /**
  * ecryptfs_decrypt_page_offset
+ * @crypt_stat: The cryptographic context
+ * @dst_page: The page to decrypt into
+ * @dst_offset: The offset in the page to decrypt into
+ * @src_page: The page to decrypt from
+ * @src_offset: The offset in the page to decrypt from
+ * @size: The number of bytes to decrypt
+ * @iv: The initialization vector to use for the decryption
  *
  * Returns the number of bytes decrypted
  */
@@ -940,6 +959,8 @@ static void ecryptfs_generate_new_key(struct ecryptfs_crypt_stat *crypt_stat)
 
 /**
  * ecryptfs_copy_mount_wide_flags_to_inode_flags
+ * @crypt_stat: The inode's cryptographic context
+ * @mount_crypt_stat: The mount point's cryptographic context
  *
  * This function propagates the mount-wide flags to individual inode
  * flags.
@@ -980,7 +1001,8 @@ out:
 
 /**
  * ecryptfs_set_default_crypt_stat_vals
- * @crypt_stat
+ * @crypt_stat: The inode's cryptographic context
+ * @mount_crypt_stat: The mount point's cryptographic context
  *
  * Default values in the event that policy does not override them.
  */
@@ -1000,7 +1022,7 @@ static void ecryptfs_set_default_crypt_stat_vals(
 
 /**
  * ecryptfs_new_file_context
- * @ecryptfs_dentry
+ * @ecryptfs_dentry: The eCryptfs dentry
  *
  * If the crypto context for the file has not yet been established,
  * this is where we do that.  Establishing a new crypto context
@@ -1017,7 +1039,6 @@ static void ecryptfs_set_default_crypt_stat_vals(
  *
  * Returns zero on success; non-zero otherwise
  */
-/* Associate an authentication token(s) with the file */
 int ecryptfs_new_file_context(struct dentry *ecryptfs_dentry)
 {
 	struct ecryptfs_crypt_stat *crypt_stat =
@@ -1095,7 +1116,7 @@ static struct ecryptfs_flag_map_elem ecryptfs_flag_map[] = {
 
 /**
  * ecryptfs_process_flags
- * @crypt_stat
+ * @crypt_stat: The cryptographic context
  * @page_virt: Source data to be parsed
  * @bytes_read: Updated with the number of bytes read
  *
@@ -1183,7 +1204,7 @@ ecryptfs_cipher_code_str_map[] = {
 
 /**
  * ecryptfs_code_for_cipher_string
- * @str: The string representing the cipher name
+ * @crypt_stat: The cryptographic context
  *
  * Returns zero on no match, or the cipher code on match
  */
@@ -1241,9 +1262,9 @@ int ecryptfs_cipher_code_to_string(char *str, u16 cipher_code)
 
 /**
  * ecryptfs_read_header_region
- * @data
- * @dentry
- * @nd
+ * @data: The virtual address to write header region data into
+ * @dentry: The lower dentry
+ * @mnt: The lower VFS mount
  *
  * Returns zero on success; non-zero otherwise
  */
@@ -1315,9 +1336,10 @@ struct kmem_cache *ecryptfs_header_cache_2;
 
 /**
  * ecryptfs_write_headers_virt
- * @page_virt
- * @crypt_stat
- * @ecryptfs_dentry
+ * @page_virt: The virtual address to write the headers to
+ * @size: Set to the number of bytes written by this function
+ * @crypt_stat: The cryptographic context
+ * @ecryptfs_dentry: The eCryptfs dentry
  *
  * Format version: 1
  *
@@ -1371,9 +1393,9 @@ static int ecryptfs_write_headers_virt(char *page_virt, size_t *size,
 	return rc;
 }
 
-static int ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat,
-					       struct file *lower_file,
-					       char *page_virt)
+static int
+ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat,
+				    struct file *lower_file, char *page_virt)
 {
 	mm_segment_t oldfs;
 	int current_header_page;
@@ -1415,9 +1437,10 @@ out:
 	return rc;
 }
 
-static int ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
-					    struct ecryptfs_crypt_stat *crypt_stat,
-					    char *page_virt, size_t size)
+static int
+ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
+				 struct ecryptfs_crypt_stat *crypt_stat,
+				 char *page_virt, size_t size)
 {
 	int rc;
 
@@ -1428,6 +1451,7 @@ static int ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
 
 /**
  * ecryptfs_write_metadata
+ * @ecryptfs_dentry: The eCryptfs dentry
  * @lower_file: The lower file struct, which was returned from dentry_open
  *
  * Write the file headers out.  This will likely involve a userspace
@@ -1525,6 +1549,7 @@ static int parse_header_metadata(struct ecryptfs_crypt_stat *crypt_stat,
 
 /**
  * set_default_header_data
+ * @crypt_stat: The cryptographic context
  *
  * For version 0 file format; this function is only for backwards
  * compatibility for files created with the prior versions of
@@ -1538,6 +1563,10 @@ static void set_default_header_data(struct ecryptfs_crypt_stat *crypt_stat)
 
 /**
  * ecryptfs_read_headers_virt
+ * @page_virt: The virtual address into which to read the headers
+ * @crypt_stat: The cryptographic context
+ * @ecryptfs_dentry: The eCryptfs dentry
+ * @validate_header_size: Whether to validate the header size while reading
  *
  * Read/parse the header data. The header format is detailed in the
  * comment block for the ecryptfs_write_headers_virt() function.
@@ -1597,9 +1626,13 @@ out:
 
 /**
  * ecryptfs_read_xattr_region
+ * @page_virt: The vitual address into which to read the xattr data
+ * @ecryptfs_dentry: The eCryptfs dentry
  *
  * Attempts to read the crypto metadata from the extended attribute
  * region of the lower file.
+ *
+ * Returns zero on success; non-zero on error
  */
 int ecryptfs_read_xattr_region(char *page_virt, struct dentry *ecryptfs_dentry)
 {
@@ -1638,6 +1671,8 @@ out:
 
 /**
  * ecryptfs_read_metadata
+ * @ecryptfs_dentry: The eCryptfs dentry
+ * @lower_file: The lower file from which to read the metadata
  *
  * Common entry point for reading file metadata. From here, we could
  * retrieve the header information from the header region of the file,
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index 8485e77..a718136 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -71,7 +71,7 @@ int process_request_key_err(long err_code)
  *        address; zero on error
  * @length_size: The number of bytes occupied by the encoded length
  *
- * Returns Zero on success
+ * Returns zero on success; non-zero on error
  */
 static int parse_packet_length(unsigned char *data, size_t *size,
 			       size_t *length_size)
@@ -106,11 +106,11 @@ out:
 
 /**
  * write_packet_length
- * @dest: The byte array target into which to write the
- *       length. Must have at least 5 bytes allocated.
+ * @dest: The byte array target into which to write the length. Must
+ *        have at least 5 bytes allocated.
  * @size: The length to write.
- * @packet_size_length: The number of bytes used to encode the
- *                      packet length is written to this address.
+ * @packet_size_length: The number of bytes used to encode the packet
+ *                      length is written to this address.
  *
  * Returns zero on success; non-zero on error.
  */
@@ -397,10 +397,11 @@ out:
 }
 
 /**
- * decrypt_pki_encrypted_session_key - Decrypt the session key with
- * the given auth_tok.
+ * decrypt_pki_encrypted_session_key - Decrypt the session key with the given auth_tok.
+ * @auth_tok: The key authentication token used to decrypt the session key
+ * @crypt_stat: The cryptographic context
  *
- * Returns Zero on success; non-zero error otherwise.
+ * Returns zero on success; non-zero error otherwise.
  */
 static int
 decrypt_pki_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
@@ -484,18 +485,18 @@ struct kmem_cache *ecryptfs_auth_tok_list_item_cache;
 
 /**
  * parse_tag_1_packet
- * @crypt_stat: The cryptographic context to modify based on packet
- *              contents.
+ * @crypt_stat: The cryptographic context to modify based on packet contents
  * @data: The raw bytes of the packet.
  * @auth_tok_list: eCryptfs parses packets into authentication tokens;
- *                 a new authentication token will be placed at the end
- *                 of this list for this packet.
+ *                 a new authentication token will be placed at the
+ *                 end of this list for this packet.
  * @new_auth_tok: Pointer to a pointer to memory that this function
  *                allocates; sets the memory address of the pointer to
  *                NULL on error. This object is added to the
  *                auth_tok_list.
  * @packet_size: This function writes the size of the parsed packet
  *               into this memory location; zero on error.
+ * @max_packet_size: The maximum allowable packet size
  *
  * Returns zero on success; non-zero on error.
  */
@@ -996,10 +997,11 @@ ecryptfs_find_auth_tok_for_sig(
 }
 
 /**
- * decrypt_passphrase_encrypted_session_key - Decrypt the session key
- * with the given auth_tok.
+ * decrypt_passphrase_encrypted_session_key - Decrypt the session key with the given auth_tok.
+ * @auth_tok: The passphrase authentication token to use to encrypt the FEK
+ * @crypt_stat: The cryptographic context
  *
- * Returns Zero on success; non-zero error otherwise.
+ * Returns zero on success; non-zero error otherwise
  */
 static int
 decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
@@ -1102,8 +1104,9 @@ int ecryptfs_get_auth_tok_sig(char **sig, struct ecryptfs_auth_tok *auth_tok)
 
 /**
  * ecryptfs_parse_packet_set
- * @dest: The header page in memory
- * @version: Version of file format, to guide parsing behavior
+ * @crypt_stat: The cryptographic context
+ * @src: Virtual address of region of memory containing the packets
+ * @ecryptfs_dentry: The eCryptfs dentry associated with the packet set
  *
  * Get crypt_stat to have the file's session key if the requisite key
  * is available to decrypt the session key.
@@ -1354,7 +1357,10 @@ out:
 /**
  * write_tag_1_packet - Write an RFC2440-compatible tag 1 (public key) packet
  * @dest: Buffer into which to write the packet
- * @max: Maximum number of bytes that can be writtn
+ * @remaining_bytes: Maximum number of bytes that can be writtn
+ * @auth_tok: The authentication token used for generating the tag 1 packet
+ * @crypt_stat: The cryptographic context
+ * @key_rec: The key record struct for the tag 1 packet
  * @packet_size: This function will write the number of bytes that end
  *               up constituting the packet; set to zero on error
  *
@@ -1441,7 +1447,7 @@ out:
 /**
  * write_tag_11_packet
  * @dest: Target into which Tag 11 packet is to be written
- * @max: Maximum packet length
+ * @remaining_bytes: Maximum packet length
  * @contents: Byte array of contents to copy in
  * @contents_length: Number of bytes in contents
  * @packet_length: Length of the Tag 11 packet written; zero on error
@@ -1501,7 +1507,7 @@ write_tag_11_packet(char *dest, int *remaining_bytes, char *contents,
 /**
  * write_tag_3_packet
  * @dest: Buffer into which to write the packet
- * @max: Maximum number of bytes that can be written
+ * @remaining_bytes: Maximum number of bytes that can be written
  * @auth_tok: Authentication token
  * @crypt_stat: The cryptographic context
  * @key_rec: encrypted key
@@ -1707,7 +1713,7 @@ struct kmem_cache *ecryptfs_key_record_cache;
 
 /**
  * ecryptfs_generate_key_packet_set
- * @dest: Virtual address from which to write the key record set
+ * @dest_base: Virtual address from which to write the key record set
  * @crypt_stat: The cryptographic context from which the
  *              authentication tokens will be retrieved
  * @ecryptfs_dentry: The dentry, used to retrieve the mount crypt stat
-- 
1.5.1.6

[PATCH 6/7] eCryptfs: Remove unnecessary variable initializations

[Michael Halcrow]

Andrew Morton wrote:
> >       struct mutex *tfm_mutex = NULL;
>
> This initialisation looks like it's here to kill bogus gcc warning
> (if it is, it should have been commented).  Please investigate
> uninitialized_var() and __maybe_unused sometime.

Remove some unnecessary variable initializations. There may be a few
more such intializations remaining in the code base; a future patch
will take care of those.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/keystore.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index a718136..d9739bc 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -1009,7 +1009,7 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
 {
 	struct scatterlist dst_sg;
 	struct scatterlist src_sg;
-	struct mutex *tfm_mutex = NULL;
+	struct mutex *tfm_mutex;
 	struct blkcipher_desc desc = {
 		.flags = CRYPTO_TFM_REQ_MAY_SLEEP
 	};
@@ -1123,8 +1123,8 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 	size_t found_auth_tok;
 	size_t next_packet_is_auth_tok_packet;
 	struct list_head auth_tok_list;
-	struct ecryptfs_auth_tok *matching_auth_tok = NULL;
-	struct ecryptfs_auth_tok *candidate_auth_tok = NULL;
+	struct ecryptfs_auth_tok *matching_auth_tok;
+	struct ecryptfs_auth_tok *candidate_auth_tok;
 	char *candidate_auth_tok_sig;
 	size_t packet_size;
 	struct ecryptfs_auth_tok *new_auth_tok;
-- 
1.5.1.6

[PATCH 7/7] eCryptfs: Make needlessly global symbols static

[Michael Halcrow]

Andrew Morton wrote:
> Please check that all the newly-added global symbols do indeed need
> to be global.

Change symbols in keystore.c and crypto.o to static if they do not
need to be global.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>
---
 fs/ecryptfs/crypto.c          |    8 +++---
 fs/ecryptfs/ecryptfs_kernel.h |    7 ------
 fs/ecryptfs/keystore.c        |   43 +++++++++++++++++++++--------------------
 3 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 7aa2f48..8e9b36d 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -123,9 +123,9 @@ out:
 	return rc;
 }
 
-int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
-					   char *cipher_name,
-					   char *chaining_modifier)
+static int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
+						  char *cipher_name,
+						  char *chaining_modifier)
 {
 	int cipher_name_len = strlen(cipher_name);
 	int chaining_modifier_len = strlen(chaining_modifier);
@@ -1859,7 +1859,7 @@ out:
  * should be released by other functions, such as on a superblock put
  * event, regardless of whether this function succeeds for fails.
  */
-int
+static int
 ecryptfs_process_key_cipher(struct crypto_blkcipher **key_tfm,
 			    char *cipher_name, size_t *key_size)
 {
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e09e1fb..6cd1e18 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -156,7 +156,6 @@ struct ecryptfs_auth_tok {
 	} token;
 } __attribute__ ((packed));
 
-int ecryptfs_get_auth_tok_sig(char **sig, struct ecryptfs_auth_tok *auth_tok);
 void ecryptfs_dump_auth_tok(struct ecryptfs_auth_tok *auth_tok);
 extern void ecryptfs_to_hex(char *dst, char *src, size_t src_size);
 extern void ecryptfs_from_hex(char *dst, char *src, int dst_size);
@@ -536,9 +535,6 @@ void ecryptfs_destroy_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat);
 void ecryptfs_destroy_mount_crypt_stat(
 	struct ecryptfs_mount_crypt_stat *mount_crypt_stat);
 int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat);
-int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
-					   char *cipher_name,
-					   char *chaining_modifier);
 #define ECRYPTFS_LOWER_I_MUTEX_NOT_HELD 0
 #define ECRYPTFS_LOWER_I_MUTEX_HELD 1
 int ecryptfs_write_inode_size_to_metadata(struct file *lower_file,
@@ -579,13 +575,10 @@ int ecryptfs_generate_key_packet_set(char *dest_base,
 				     struct ecryptfs_crypt_stat *crypt_stat,
 				     struct dentry *ecryptfs_dentry,
 				     size_t *len, size_t max);
-int process_request_key_err(long err_code);
 int
 ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
 			  unsigned char *src, struct dentry *ecryptfs_dentry);
 int ecryptfs_truncate(struct dentry *dentry, loff_t new_length);
-int ecryptfs_process_key_cipher(struct crypto_blkcipher **key_tfm,
-				char *cipher_name, size_t *key_size);
 int ecryptfs_inode_test(struct inode *inode, void *candidate_lower_inode);
 int ecryptfs_inode_set(struct inode *inode, void *lower_inode);
 void ecryptfs_init_inode(struct inode *inode, struct inode *lower_inode);
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index d9739bc..63c6ded 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -39,7 +39,7 @@
  * determine the type of error, make appropriate log entries, and
  * return an error code.
  */
-int process_request_key_err(long err_code)
+static int process_request_key_err(long err_code)
 {
 	int rc = 0;
 
@@ -396,6 +396,27 @@ out:
 	return rc;
 }
 
+static int
+ecryptfs_get_auth_tok_sig(char **sig, struct ecryptfs_auth_tok *auth_tok)
+{
+	int rc = 0;
+
+	(*sig) = NULL;
+	switch (auth_tok->token_type) {
+	case ECRYPTFS_PASSWORD:
+		(*sig) = auth_tok->token.password.signature;
+		break;
+	case ECRYPTFS_PRIVATE_KEY:
+		(*sig) = auth_tok->token.private_key.signature;
+		break;
+	default:
+		printk(KERN_ERR "Cannot get sig for auth_tok of type [%d]\n",
+		       auth_tok->token_type);
+		rc = -EINVAL;
+	}
+	return rc;
+}
+
 /**
  * decrypt_pki_encrypted_session_key - Decrypt the session key with the given auth_tok.
  * @auth_tok: The key authentication token used to decrypt the session key
@@ -1082,26 +1103,6 @@ out:
 	return rc;
 }
 
-int ecryptfs_get_auth_tok_sig(char **sig, struct ecryptfs_auth_tok *auth_tok)
-{
-	int rc = 0;
-
-	(*sig) = NULL;
-	switch (auth_tok->token_type) {
-	case ECRYPTFS_PASSWORD:
-		(*sig) = auth_tok->token.password.signature;
-		break;
-	case ECRYPTFS_PRIVATE_KEY:
-		(*sig) = auth_tok->token.private_key.signature;
-		break;
-	default:
-		printk(KERN_ERR "Cannot get sig for auth_tok of type [%d]\n",
-		       auth_tok->token_type);
-		rc = -EINVAL;
-	}
-	return rc;
-}
-
 /**
  * ecryptfs_parse_packet_set
  * @crypt_stat: The cryptographic context
-- 
1.5.1.6

Re: [PATCH 4/7] eCryptfs: Comments for some structs

[Satyam Sharma]

Trivial nits ...

On 7/26/07, Michael Halcrow <mhalcrow@us.ibm.com> wrote:
> [...]
> +/**
> + * ecryptfs_global_auth_tok structs refer to authentication token keys
> + * in the user keyring that apply to newly created files. A list of
> + * these objects hangs off of the mount_crypt_stat struct for any
> + * given eCryptfs mount. This struct maintains a reference to both the
> + * key contents and the key itself so that the key can be put on
> + * unmount.
> + */

/** is used to annotate kernel-doc style comments, which this
one isn't -- IIRC, kernel-doc doesn't like this (?)

>  struct ecryptfs_global_auth_tok {
>  #define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
>         u32 flags;
> -       struct list_head mount_crypt_stat_list;
> -       struct key *global_auth_tok_key;
> -       struct ecryptfs_auth_tok *global_auth_tok;
> -       unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1];

> +       struct list_head mount_crypt_stat_list; /* Default auth_tok list for
> +                                                * the mount_crypt_stat */
> +       struct key *global_auth_tok_key; /* The key from the user's keyring for
> +                                         * the sig */

Tsk. You could consider using kernel-doc style itself to comment the
structure -- this stuff goes up there and doesn't look icky.

> +       struct ecryptfs_auth_tok *global_auth_tok; /* The key contents */
> +       unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1]; /* The key identifier */
>  };
>
> +/**
> + * Typically, eCryptfs will use the same ciphers repeatedly throughout
> + * the course of its operations. In order to avoid unnecessarily
> + * destroying and initializing the same cipher repeatedly, eCryptfs
> + * keeps a list of crypto API contexts around to use when needed.
> + */

Again, you could consider using kernel-doc style comments here.

>  struct ecryptfs_key_tfm {
>         struct crypto_blkcipher *key_tfm;
>         size_t key_size;
>         struct mutex key_tfm_mutex;
> -       struct list_head key_tfm_list;
> +       struct list_head key_tfm_list; /* The module's tfm list */
>         unsigned char cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE + 1];
>  };


Satyam