linux-ima-devel — Discussion list for problems or development.

[Linux-ima-devel] Write a file in to overlayfs will get stucked in linux-3.18 with IMA fix mode

[Rock L. <roc...@gm...>]

Hi,

Write a file into a overlayfs may cause the process get stucked, this
is because in IMA fix mode, IMA will fix the xattr of files.
ima_check_last_writer() holds inode->i_mutex, and call
__vfs_setxattr_noperm(). It works with most filesystems. But not
overlayfs, overlayfs calls vfs_setxattr which also holds
inode->i_mutex, when works with IMA fix mode, process will get
stucked.

But for the recent linux-4.13, there is no such problem, VFS changed a
lot. I wrote a patch for linux-3.18 to make overlayfs works in IMA fix
mode. It works, but I don't know if it is reasonable, could you give
some advices ?

 security/integrity/ima/ima_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 62f59ec..171fe9b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -120,7 +120,12 @@ static void ima_check_last_writer(struct
integrity_iint_cache *iint,
   if (!(mode & FMODE_WRITE))
       return;
-   mutex_lock(&inode->i_mutex);
+   /*
+    * For overlayfs, it calls vfs_setxattr which holds inode->i_mutex,
+    * so, don't lock inode.
+    */
+   if (inode == file->f_dentry->d_inode)
+       mutex_lock(&inode->i_mutex);
   if (atomic_read(&inode->i_writecount) == 1) {
       if ((iint->version != inode->i_version) ||
        (iint->flags & IMA_NEW_FILE)) {
@@ -129,7 +134,8 @@ static void ima_check_last_writer(struct
integrity_iint_cache *iint,
               ima_update_xattr(iint, file);
       }
   }
-   mutex_unlock(&inode->i_mutex);
+   if (inode == file->f_dentry->d_inode)
+       mutex_unlock(&inode->i_mutex);
}
/**
-- 
1.9.1

-- 
Cheers,
Rock

[Linux-ima-devel] Write a file into nfs will get stucked

[Rock L. <roc...@gm...>]

Hi,

I enabled IMA, but when I write a file into nfs, the process will get
stucked. I've trace the code, it seems it never return in
ima_calc_file_shash(). Could IMA work with nfs ??

BTW, I am using raspberrypi3, linux-4.13, with IMA enabled. And my
kernel cmdline uses "ima_tcb ima_appraise=fix ima_appraise_tcb".

-- 
Cheers,
Rock

Re: [Linux-ima-devel] [tpmdd-devel] [PATCH v2 0/4] additional TPM performance improvements

[<Ale...@in...>]

> After further discussions with the Device Driver working group (ddwg),
> the following changes were made:
> 
> * Check for burstcount at least once to confirm the TPM is ready to accept
> the data. Similarly, query for the TPM Expect status as sanity check at
> the end.
> 
> * Make the sleep for status check during send() in the loop less than
> 5msec.
> 
> * Make the sleep in the loop while querying for burstcount less than
> 5msec.
> 
> Below is the list of patches along with the performance improvements
> seen with a TPM 1.2 with an 8 byte burstcount for 1000 extends:
> 
> Patch                                        |Improvement(time in sec)
> 
> tpm: ignore burstcount to improve tpm_tis    | ~41 - ~14
> send() performance.
> 
> tpm: define __wait_for_tpm_stat to specify   | ~14 - ~10
> variable polling sleep time
> 
> tpm: reduce tpm_msleep() time in             | ~10 - ~9
> get_burstcount()
> 
> tpm: modify tpm_msleep() function to have    | ~9 - ~8
> max range
> 
> Changelog v2:
> 
> * Add module parameter to handle ignoring of burst count during
> tpm tis send() operation.
> * Add improvements over sleep time to reduce delays.
> 
> Nayna Jain (4):
>   tpm: ignore burstcount to improve tpm_tis send() performance.
>   tpm: define __wait_for_tpm_stat to specify variable polling sleep time
>   tpm: reduce tpm_msleep() time in get_burstcount()
>   tpm: use tpm_msleep() value as max delay
> 
>  Documentation/admin-guide/kernel-parameters.txt |  8 ++++++
>  drivers/char/tpm/tpm-interface.c                | 15 ++++++++--
>  drivers/char/tpm/tpm.h                          |  7 +++--
>  drivers/char/tpm/tpm_tis_core.c                 | 37 +++++++++++++++++++------
>  4 files changed, 53 insertions(+), 14 deletions(-)
> 
> --
> 2.13.3

I haven't looked at the changes in detail yet, but some initial comments:

The ignore_burst_count functionality has already received some negative feedback and probably needs more iterations, if it can be accepted at all, so you might want to split it off from the rest of the series.

Also, did you explore any alternative ways to activate that functionality besides a command line parameter? If it is off by default, then most users will never see the benefits, and if they do activate it manually, then they might hit some obscure bugs because those code paths receive only little testing. I could imagine for example activating this functionality automatically for certain safe drivers like tpm_tis_spi, where wait states cannot block us (or anyone else on the bus) indefinitely.

I ran all your patches through my tests (though without making any changes, so ignore_burst_count was off, since that is the default) and did not see any failures. My test bench does not contain performance tests (yet), so I cannot confirm your measurements. checkpatch.pl has a couple of complaints though.

Alexander

[Linux-ima-devel] [PATCH] IMA: Add support for IMA validation after credentials are committed

[Matthew G. <mj...@go...>]

It may be desirable to perform appraisal after credentials are
committed, for instance in the case where validation is only required if
the binary has transitioned into a privileged security context. Add an
additional call into IMA in the committed_credentials security hook and
abort execution if it fails.

Signed-off-by: Matthew Garrett <mj...@go...>
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 arch/x86/ia32/ia32_aout.c             |  4 +++-
 fs/binfmt_aout.c                      |  4 +++-
 fs/binfmt_elf.c                       |  5 ++++-
 fs/binfmt_elf_fdpic.c                 |  5 ++++-
 fs/binfmt_flat.c                      |  4 +++-
 fs/exec.c                             |  8 ++++++--
 include/linux/binfmts.h               |  2 +-
 include/linux/ima.h                   |  6 ++++++
 include/linux/security.h              |  5 +++--
 security/integrity/iint.c             |  1 +
 security/integrity/ima/ima.h          |  1 +
 security/integrity/ima/ima_api.c      |  2 +-
 security/integrity/ima/ima_appraise.c |  8 ++++++++
 security/integrity/ima/ima_main.c     | 24 +++++++++++++++++++++++-
 security/integrity/ima/ima_policy.c   |  4 ++++
 security/integrity/integrity.h        |  9 +++++++--
 security/security.c                   |  3 ++-
 18 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..5dc9eed035fb 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,7 +25,7 @@ Description:
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [permit_directio]
 
-		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
+		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index 8d0879f1d42c..5eaedc31661b 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -312,7 +312,9 @@ static int load_aout_binary(struct linux_binprm *bprm)
 	if (retval < 0)
 		return retval;
 
-	install_exec_creds(bprm);
+	retval = install_exec_creds(bprm);
+	if (retval)
+		return retval;
 
 	if (N_MAGIC(ex) == OMAGIC) {
 		unsigned long text_addr, map_size;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 9be82c4e14a4..5eb778710a46 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -256,7 +256,9 @@ static int load_aout_binary(struct linux_binprm * bprm)
 	if (retval < 0)
 		return retval;
 
-	install_exec_creds(bprm);
+	retval = install_exec_creds(bprm);
+	if (retval)
+		return retval;
 
 	if (N_MAGIC(ex) == OMAGIC) {
 		unsigned long text_addr, map_size;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6466153f2bf0..0f0463e8bcb8 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -865,7 +865,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		current->flags |= PF_RANDOMIZE;
 
 	setup_new_exec(bprm);
-	install_exec_creds(bprm);
+
+	retval = install_exec_creds(bprm);
+	if (retval)
+		goto out_free_dentry;
 
 	/* Do this so that we can load the interpreter, if need be.  We will
 	   change some of these later */
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index cf93a4fad012..066f81d31d7b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -432,7 +432,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 	current->mm->start_stack = current->mm->start_brk + stack_size;
 #endif
 
-	install_exec_creds(bprm);
+	retval = install_exec_creds(bprm);
+	if (retval)
+		goto error;
+
 	if (create_elf_fdpic_tables(bprm, current->mm,
 				    &exec_params, &interp_params) < 0)
 		goto error;
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a1e6860b6f46..61cc1099d8a6 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -958,7 +958,9 @@ static int load_flat_binary(struct linux_binprm *bprm)
 		}
 	}
 
-	install_exec_creds(bprm);
+	retval = install_exec_creds(bprm);
+	if (retval)
+		return retval;
 
 	set_binfmt(&flat_format);
 
diff --git a/fs/exec.c b/fs/exec.c
index 62175cbcc801..8923f0ce5d57 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1418,8 +1418,10 @@ EXPORT_SYMBOL(bprm_change_interp);
 /*
  * install the new credentials for this executable
  */
-void install_exec_creds(struct linux_binprm *bprm)
+int install_exec_creds(struct linux_binprm *bprm)
 {
+	int ret = 0;
+
 	security_bprm_committing_creds(bprm);
 
 	commit_creds(bprm->cred);
@@ -1438,8 +1440,10 @@ void install_exec_creds(struct linux_binprm *bprm)
 	 * ptrace_attach() from altering our determination of the task's
 	 * credentials; any time after this it may be unlocked.
 	 */
-	security_bprm_committed_creds(bprm);
+	ret = security_bprm_committed_creds(bprm);
 	mutex_unlock(&current->signal->cred_guard_mutex);
+
+	return ret;
 }
 EXPORT_SYMBOL(install_exec_creds);
 
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 3ae9013eeaaa..4d60d2c432d9 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -121,7 +121,7 @@ extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
 extern int copy_strings_kernel(int argc, const char *const *argv,
 			       struct linux_binprm *bprm);
 extern int prepare_bprm_creds(struct linux_binprm *bprm);
-extern void install_exec_creds(struct linux_binprm *bprm);
+extern int install_exec_creds(struct linux_binprm *bprm);
 extern void set_binfmt(struct linux_binfmt *new);
 extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..f9a64f94b0d3 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -16,6 +16,7 @@ struct linux_binprm;
 
 #ifdef CONFIG_IMA
 extern int ima_bprm_check(struct linux_binprm *bprm);
+extern int ima_creds_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
@@ -34,6 +35,11 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
 	return 0;
 }
 
+static inline int ima_creds_check(struct linux_binprm *bprm)
+{
+	return 0;
+}
+
 static inline int ima_file_check(struct file *file, int mask, int opened)
 {
 	return 0;
diff --git a/include/linux/security.h b/include/linux/security.h
index b6ea1dc9cc9d..21763448dc89 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -231,7 +231,7 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
 int security_bprm_set_creds(struct linux_binprm *bprm);
 int security_bprm_check(struct linux_binprm *bprm);
 void security_bprm_committing_creds(struct linux_binprm *bprm);
-void security_bprm_committed_creds(struct linux_binprm *bprm);
+int security_bprm_committed_creds(struct linux_binprm *bprm);
 int security_bprm_secureexec(struct linux_binprm *bprm);
 int security_sb_alloc(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
@@ -537,8 +537,9 @@ static inline void security_bprm_committing_creds(struct linux_binprm *bprm)
 {
 }
 
-static inline void security_bprm_committed_creds(struct linux_binprm *bprm)
+static inline int security_bprm_committed_creds(struct linux_binprm *bprm)
 {
+	return 0;
 }
 
 static inline int security_bprm_secureexec(struct linux_binprm *bprm)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 6fc888ca468e..ad30094a58b4 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint)
 	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
 	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
 	iint->ima_read_status = INTEGRITY_UNKNOWN;
+	iint->ima_creds_status = INTEGRITY_UNKNOWN;
 	iint->evm_status = INTEGRITY_UNKNOWN;
 	iint->measured_pcrs = 0;
 	kmem_cache_free(iint_cache, iint);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..547ea832bb1b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(FILE_CHECK)		\
 	hook(MMAP_CHECK)		\
 	hook(BPRM_CHECK)		\
+	hook(CREDS_CHECK)		\
 	hook(POST_SETATTR)		\
 	hook(MODULE_CHECK)		\
 	hook(FIRMWARE_CHECK)		\
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8de35e..0c19bb423570 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -165,7 +165,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
- *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
+ *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 809ba70fbbbf..edb82e722a0d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -86,6 +86,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_mmap_status;
 	case BPRM_CHECK:
 		return iint->ima_bprm_status;
+	case CREDS_CHECK:
+		return iint->ima_creds_status;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return iint->ima_file_status;
@@ -106,6 +108,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->ima_bprm_status = status;
 		break;
+	case CREDS_CHECK:
+		iint->ima_creds_status = status;
+		break;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->ima_file_status = status;
@@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
 		break;
+	case CREDS_CHECK:
+		iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED);
+		break;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..5be8307a1dd1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -14,7 +14,7 @@
  *
  * File: ima_main.c
  *	implements the IMA hooks: ima_bprm_check, ima_file_mmap,
- *	and ima_file_check.
+ *	ima_creds_check and ima_file_check.
  */
 #include <linux/module.h>
 #include <linux/file.h>
@@ -306,6 +306,28 @@ int ima_bprm_check(struct linux_binprm *bprm)
 				   BPRM_CHECK, 0);
 }
 
+/**
+ * ima_creds_check - based on policy, collect/store measurement.
+ * @bprm: contains the linux_binprm structure
+ *
+ * The OS protects against an executable file, already open for write,
+ * from being executed in deny_write_access() and an executable file,
+ * already open for execute, from being modified in get_write_access().
+ * So we can be certain that what we verify and measure here is actually
+ * what is being executed.
+ *
+ * This is identical to ima_bprm_check, except called after child credentials
+ * have been committed.
+ *
+ * On success return 0.  On integrity appraisal error, assuming the file
+ * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
+ */
+int ima_creds_check(struct linux_binprm *bprm)
+{
+	return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
+				   CREDS_CHECK, 0);
+}
+
 /**
  * ima_path_check - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 95209a5f8595..a6e14c532627 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -339,6 +339,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 		return IMA_MMAP_APPRAISE;
 	case BPRM_CHECK:
 		return IMA_BPRM_APPRAISE;
+	case CREDS_CHECK:
+		return IMA_CREDS_APPRAISE;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return IMA_FILE_APPRAISE;
@@ -691,6 +693,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
 				entry->func = BPRM_CHECK;
+			else if (strcmp(args[0].from, "CREDS_CHECK") == 0)
+				entry->func = CREDS_CHECK;
 			else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
 				 0)
 				entry->func = KEXEC_KERNEL_CHECK;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..45ba0e4501d6 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -48,10 +48,14 @@
 #define IMA_BPRM_APPRAISED	0x00002000
 #define IMA_READ_APPRAISE	0x00004000
 #define IMA_READ_APPRAISED	0x00008000
+#define IMA_CREDS_APPRAISE	0x00010000
+#define IMA_CREDS_APPRAISED	0x00020000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
-				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
+				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
+				 IMA_CREDS_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
-				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
+				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
+				 IMA_CREDS_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,
@@ -108,6 +112,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_mmap_status:4;
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_read_status:4;
+	enum integrity_status ima_creds_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;
 };
diff --git a/security/security.c b/security/security.c
index 30132378d103..bdb5cd5c8859 100644
--- a/security/security.c
+++ b/security/security.c
@@ -346,9 +346,10 @@ void security_bprm_committing_creds(struct linux_binprm *bprm)
 	call_void_hook(bprm_committing_creds, bprm);
 }
 
-void security_bprm_committed_creds(struct linux_binprm *bprm)
+int security_bprm_committed_creds(struct linux_binprm *bprm)
 {
 	call_void_hook(bprm_committed_creds, bprm);
+	return ima_creds_check(bprm);
 }
 
 int security_bprm_secureexec(struct linux_binprm *bprm)
-- 
2.14.1.581.gf28d330327-goog

[Linux-ima-devel] [SPAM] A first-in-class logo and website for your business (IW)

[Pamela M. <pam...@te...>]

An HTML attachment was scrubbed...

Re: [Linux-ima-devel] [PATCH v2 0/4] additional TPM performance improvements

[Jarkko S. <jar...@li...>]

On Wed, Sep 06, 2017 at 08:56:35AM -0400, Nayna Jain wrote:
> After further discussions with the Device Driver working group (ddwg),
> the following changes were made: 
> 
> * Check for burstcount at least once to confirm the TPM is ready to accept
> the data. Similarly, query for the TPM Expect status as sanity check at
> the end.
> 
> * Make the sleep for status check during send() in the loop less than
> 5msec.
> 
> * Make the sleep in the loop while querying for burstcount less than
> 5msec.
> 
> Below is the list of patches along with the performance improvements
> seen with a TPM 1.2 with an 8 byte burstcount for 1000 extends: 
> 
> Patch                                        |Improvement(time in sec)
> 
> tpm: ignore burstcount to improve tpm_tis    | ~41 - ~14
> send() performance.
> 
> tpm: define __wait_for_tpm_stat to specify   | ~14 - ~10
> variable polling sleep time
> 
> tpm: reduce tpm_msleep() time in             | ~10 - ~9
> get_burstcount()
> 
> tpm: modify tpm_msleep() function to have    | ~9 - ~8
> max range
> 
> Changelog v2:
> 
> * Add module parameter to handle ignoring of burst count during
> tpm tis send() operation.
> * Add improvements over sleep time to reduce delays.
> 
> Nayna Jain (4):
>   tpm: ignore burstcount to improve tpm_tis send() performance.
>   tpm: define __wait_for_tpm_stat to specify variable polling sleep time
>   tpm: reduce tpm_msleep() time in get_burstcount()
>   tpm: use tpm_msleep() value as max delay
> 
>  Documentation/admin-guide/kernel-parameters.txt |  8 ++++++
>  drivers/char/tpm/tpm-interface.c                | 15 ++++++++--
>  drivers/char/tpm/tpm.h                          |  7 +++--
>  drivers/char/tpm/tpm_tis_core.c                 | 37 +++++++++++++++++++------
>  4 files changed, 53 insertions(+), 14 deletions(-)
> 
> -- 
> 2.13.3
> 

I'll look into this after next week.

/Jarkko

Re: [Linux-ima-devel] [PATCH v2 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Jason G. <jgu...@ob...>]

On Wed, Sep 06, 2017 at 08:56:36AM -0400, Nayna Jain wrote:

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 4e303be83df6..3c59bb91e1ee 100644
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1465,6 +1465,14 @@
>  			mode generally follows that for the NaN encoding,
>  			except where unsupported by hardware.
>  
> +	ignore_burst_count [TPM_TIS_CORE]
> +			tpm_tis_core driver queries for the burstcount before
> +			every send call in a loop. However, it causes delay to
> +			the send command for TPMs with low burstcount value.
> +			Setting this value to 1, will make driver to query for
> +			burstcount only once in the loop to improve the
> +			performance. By default, its value is set to 0.

Really don't want to see a kernel command line parameter for this..

Please figure out a different approach or at least a better name..

> +		/*
> +		 * Get the initial burstcount to ensure TPM is ready to
> +		 * accept data, even when waiting for burstcount is disabled.
> +		 */
>  		burstcnt = get_burstcount(chip);
>  		if (burstcnt < 0) {
>  			dev_err(&chip->dev, "Unable to read burstcount\n");
>  			rc = burstcnt;
>  			goto out_err;
>  		}
> -		burstcnt = min_t(int, burstcnt, len - count - 1);
> +
> +		if (ignore_burst_count)
> +			sendcnt = len - 1;
> +		else
> +			sendcnt = min_t(int, burstcnt, len - count - 1);
> +
>  		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> -					 burstcnt, buf + count);
> +					 sendcnt, buf + count);

The problem with this approach is that the TPM could totally block the CPU for
very long periods of time.

It seems very risky to enable..

Jason

Re: [Linux-ima-devel] About TPM support of IMA

[Mimi Z. <zo...@li...>]

On Tue, 2017-09-05 at 10:22 +0800, shijun zhao wrote:
> Hi,
> 
> Can IMA run without a TPM chip?

Yes, IMA can run in TPM-bypass mode.  Without a TPM, the measurement
list can not be quoted and verified, which kind of defeats the purpose
of IMA-measurement.

Mimi

Re: [Linux-ima-devel] About supported Linux kernel of IMA

[Mimi Z. <zo...@li...>]

On Tue, 2017-09-05 at 10:19 +0800, shijun zhao wrote:

> I only see IMA versions that support Linux kernel under 2.6.xx. My question
> is does IMA support Linux kernel 4.x? And does IMA support 64-bit OS kernel?

That's because IMA was upstreamed.

Mimi

Re: [Linux-ima-devel] [PATCH 2/4] tpm: define __wait_for_tpm_stat to specify variable polling sleep time

[Nayna <na...@li...>]

Please ignore these one.. My command took patches recursively from 
directory also.

Sorry for this.

Thanks & Regards,
- Nayna

On 09/06/2017 06:26 PM, Nayna Jain wrote:
> The existing wait_for_tpm_stat() checks the chip status before
> sleeping for 5 msec in a polling loop.  For some functions although
> the status isn't ready immediately, the status returns extremely
> quickly. Waiting for 5 msec causes an unnecessary delay. An
> example is the send() call in the tpms_tis driver.
>
> This patch defines __wait_for_tpm_stat(), allowing the caller
> to specify the polling sleep timeout value within the loop.
> The existing wait_for_tpm_stat() becomes a wrapper for this
> function.
>
> Signed-off-by: Nayna Jain <na...@li...>
> ---
>   drivers/char/tpm/tpm-interface.c | 15 ++++++++++++---
>   drivers/char/tpm/tpm.h           |  3 +++
>   drivers/char/tpm/tpm_tis_core.c  | 11 ++++++-----
>   3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1d6729be4cd6..b23d006243b7 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1050,8 +1050,9 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
>   	return false;
>   }
>
> -int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> -		      wait_queue_head_t *queue, bool check_cancel)
> +int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> +		      unsigned int poll_sleep, wait_queue_head_t *queue,
> +		      bool check_cancel)
>   {
>   	unsigned long stop;
>   	long rc;
> @@ -1085,7 +1086,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
>   		}
>   	} else {
>   		do {
> -			tpm_msleep(TPM_TIMEOUT);
> +			tpm_msleep(poll_sleep);
>   			status = chip->ops->status(chip);
>   			if ((status & mask) == mask)
>   				return 0;
> @@ -1093,6 +1094,14 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
>   	}
>   	return -ETIME;
>   }
> +EXPORT_SYMBOL_GPL(__wait_for_tpm_stat);
> +
> +int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> +		      wait_queue_head_t *queue, bool check_cancel)
> +{
> +	return __wait_for_tpm_stat(chip, mask, timeout, TPM_TIMEOUT,
> +			queue, check_cancel);
> +}
>   EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
>
>   #define TPM_ORD_SAVESTATE 152
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 2d5466a72e40..eb2f8818eded 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -525,6 +525,9 @@ int tpm_do_selftest(struct tpm_chip *chip);
>   unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>   int tpm_pm_suspend(struct device *dev);
>   int tpm_pm_resume(struct device *dev);
> +int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> +		unsigned long timeout, unsigned int poll_sleep,
> +		wait_queue_head_t *queue, bool check_cancel);
>   int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
>   		      wait_queue_head_t *queue, bool check_cancel);
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 6b9bf4c4d434..d1eab29cb447 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -268,8 +268,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>   	status = tpm_tis_status(chip);
>   	if ((status & TPM_STS_COMMAND_READY) == 0) {
>   		tpm_tis_ready(chip);
> -		if (wait_for_tpm_stat
> -		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
> +		if (__wait_for_tpm_stat
> +		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b, 1,
>   		     &priv->int_queue, false) < 0) {
>   			rc = -ETIME;
>   			goto out_err;
> @@ -303,7 +303,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>   		if (ignore_burst_count)
>   			continue;
>
> -		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> +		if (__wait_for_tpm_stat(chip, TPM_STS_VALID,
> +					chip->timeout_c, 1,
>   					&priv->int_queue, false) < 0) {
>   			rc = -ETIME;
>   			goto out_err;
> @@ -320,8 +321,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>   	if (rc < 0)
>   		goto out_err;
>
> -	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> -				&priv->int_queue, false) < 0) {
> +	if (__wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> +				1, &priv->int_queue, false) < 0) {
>   		rc = -ETIME;
>   		goto out_err;
>   	}
>

Re: [Linux-ima-devel] [PATCH 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Nayna <na...@li...>]

Please ignore these one.. My command took patches recursively from 
directory also.

Sorry for this.

Thanks & Regards,
    - Nayna

On 09/06/2017 06:26 PM, Nayna Jain wrote:
> The TPM burstcount status indicates the number of bytes that can
> be sent to the TPM without causing bus wait states.  Effectively,
> it is the number of empty bytes in the command FIFO. Further,
> some TPMs have a static burstcount, when the value remains zero
> until the entire FIFO is empty.
>
> This patch adds an optimization to check for burstcount only once.
> And if it is valid, it writes all the bytes at once, permitting
> wait states. The performance of a 34 byte extend on a TPM 1.2 with
> an 8 byte burstcount improved from 41 msec to 14 msec.
>
> This functionality is enabled only by passing module
> parameter ignore_burst_count=1. By default, this parameter
> is disabled.
>
> Suggested-by: Ken Goldman <kg...@li...> in
> conjunction with the TPM Device Driver work group.
> Signed-off-by: Nayna Jain <na...@li...>
> Acked-by: Mimi Zohar <zo...@li...>
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
>   drivers/char/tpm/tpm_tis_core.c                 | 24 +++++++++++++++++++++---
>   2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 4e303be83df6..3c59bb91e1ee 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1465,6 +1465,14 @@
>   			mode generally follows that for the NaN encoding,
>   			except where unsupported by hardware.
>
> +	ignore_burst_count [TPM_TIS_CORE]
> +			tpm_tis_core driver queries for the burstcount before
> +			every send call in a loop. However, it causes delay to
> +			the send command for TPMs with low burstcount value.
> +			Setting this value to 1, will make driver to query for
> +			burstcount only once in the loop to improve the
> +			performance. By default, its value is set to 0.
> +
>   	ignore_loglevel	[KNL]
>   			Ignore loglevel setting - this will print /all/
>   			kernel messages to the console. Useful for debugging.
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 63bc6c3b949e..6b9bf4c4d434 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -31,6 +31,11 @@
>   #include "tpm.h"
>   #include "tpm_tis_core.h"
>
> +static bool ignore_burst_count = false;
> +module_param(ignore_burst_count, bool, 0444);
> +MODULE_PARM_DESC(ignore_burst_count,
> +		"Ignore burstcount value while writing data");
> +
>   /* Before we attempt to access the TPM we must see that the valid bit is set.
>    * The specification says that this bit is 0 at reset and remains 0 until the
>    * 'TPM has gone through its self test and initialization and has established
> @@ -256,6 +261,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>   {
>   	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>   	int rc, status, burstcnt;
> +	int sendcnt;
>   	size_t count = 0;
>   	bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>
> @@ -271,19 +277,31 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>   	}
>
>   	while (count < len - 1) {
> +
> +		/*
> +		 * Get the initial burstcount to ensure TPM is ready to
> +		 * accept data, even when waiting for burstcount is disabled.
> +		 */
>   		burstcnt = get_burstcount(chip);
>   		if (burstcnt < 0) {
>   			dev_err(&chip->dev, "Unable to read burstcount\n");
>   			rc = burstcnt;
>   			goto out_err;
>   		}
> -		burstcnt = min_t(int, burstcnt, len - count - 1);
> +
> +		if (ignore_burst_count)
> +			sendcnt = len - 1;
> +		else
> +			sendcnt = min_t(int, burstcnt, len - count - 1);
> +
>   		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> -					 burstcnt, buf + count);
> +					 sendcnt, buf + count);
>   		if (rc < 0)
>   			goto out_err;
>
> -		count += burstcnt;
> +		count += sendcnt;
> +		if (ignore_burst_count)
> +			continue;
>
>   		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
>   					&priv->int_queue, false) < 0) {
>

[Linux-ima-devel] [PATCH 2/4] tpm: define __wait_for_tpm_stat to specify variable polling sleep time

[Nayna J. <na...@li...>]

The existing wait_for_tpm_stat() checks the chip status before
sleeping for 5 msec in a polling loop.  For some functions although
the status isn't ready immediately, the status returns extremely
quickly. Waiting for 5 msec causes an unnecessary delay. An
example is the send() call in the tpms_tis driver.

This patch defines __wait_for_tpm_stat(), allowing the caller
to specify the polling sleep timeout value within the loop.
The existing wait_for_tpm_stat() becomes a wrapper for this
function.

Signed-off-by: Nayna Jain <na...@li...>
---
 drivers/char/tpm/tpm-interface.c | 15 ++++++++++++---
 drivers/char/tpm/tpm.h           |  3 +++
 drivers/char/tpm/tpm_tis_core.c  | 11 ++++++-----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1d6729be4cd6..b23d006243b7 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1050,8 +1050,9 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
 	return false;
 }
 
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
-		      wait_queue_head_t *queue, bool check_cancel)
+int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+		      unsigned int poll_sleep, wait_queue_head_t *queue,
+		      bool check_cancel)
 {
 	unsigned long stop;
 	long rc;
@@ -1085,7 +1086,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 		}
 	} else {
 		do {
-			tpm_msleep(TPM_TIMEOUT);
+			tpm_msleep(poll_sleep);
 			status = chip->ops->status(chip);
 			if ((status & mask) == mask)
 				return 0;
@@ -1093,6 +1094,14 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 	}
 	return -ETIME;
 }
+EXPORT_SYMBOL_GPL(__wait_for_tpm_stat);
+
+int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+		      wait_queue_head_t *queue, bool check_cancel)
+{
+	return __wait_for_tpm_stat(chip, mask, timeout, TPM_TIMEOUT,
+			queue, check_cancel);
+}
 EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
 
 #define TPM_ORD_SAVESTATE 152
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2d5466a72e40..eb2f8818eded 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -525,6 +525,9 @@ int tpm_do_selftest(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
+int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+		unsigned long timeout, unsigned int poll_sleep,
+		wait_queue_head_t *queue, bool check_cancel);
 int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 		      wait_queue_head_t *queue, bool check_cancel);
 
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6b9bf4c4d434..d1eab29cb447 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -268,8 +268,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
 		tpm_tis_ready(chip);
-		if (wait_for_tpm_stat
-		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
+		if (__wait_for_tpm_stat
+		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b, 1,
 		     &priv->int_queue, false) < 0) {
 			rc = -ETIME;
 			goto out_err;
@@ -303,7 +303,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 		if (ignore_burst_count)
 			continue;
 
-		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+		if (__wait_for_tpm_stat(chip, TPM_STS_VALID,
+					chip->timeout_c, 1,
 					&priv->int_queue, false) < 0) {
 			rc = -ETIME;
 			goto out_err;
@@ -320,8 +321,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	if (rc < 0)
 		goto out_err;
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
+	if (__wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+				1, &priv->int_queue, false) < 0) {
 		rc = -ETIME;
 		goto out_err;
 	}
-- 
2.13.3

[Linux-ima-devel] [PATCH 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Nayna J. <na...@li...>]

The TPM burstcount status indicates the number of bytes that can
be sent to the TPM without causing bus wait states.  Effectively,
it is the number of empty bytes in the command FIFO. Further,
some TPMs have a static burstcount, when the value remains zero
until the entire FIFO is empty.

This patch adds an optimization to check for burstcount only once.
And if it is valid, it writes all the bytes at once, permitting
wait states. The performance of a 34 byte extend on a TPM 1.2 with
an 8 byte burstcount improved from 41 msec to 14 msec.

This functionality is enabled only by passing module
parameter ignore_burst_count=1. By default, this parameter
is disabled.

Suggested-by: Ken Goldman <kg...@li...> in
conjunction with the TPM Device Driver work group.
Signed-off-by: Nayna Jain <na...@li...>
Acked-by: Mimi Zohar <zo...@li...>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
 drivers/char/tpm/tpm_tis_core.c                 | 24 +++++++++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4e303be83df6..3c59bb91e1ee 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1465,6 +1465,14 @@
 			mode generally follows that for the NaN encoding,
 			except where unsupported by hardware.
 
+	ignore_burst_count [TPM_TIS_CORE]
+			tpm_tis_core driver queries for the burstcount before
+			every send call in a loop. However, it causes delay to
+			the send command for TPMs with low burstcount value.
+			Setting this value to 1, will make driver to query for
+			burstcount only once in the loop to improve the
+			performance. By default, its value is set to 0.
+
 	ignore_loglevel	[KNL]
 			Ignore loglevel setting - this will print /all/
 			kernel messages to the console. Useful for debugging.
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 63bc6c3b949e..6b9bf4c4d434 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,11 @@
 #include "tpm.h"
 #include "tpm_tis_core.h"
 
+static bool ignore_burst_count = false;
+module_param(ignore_burst_count, bool, 0444);
+MODULE_PARM_DESC(ignore_burst_count,
+		"Ignore burstcount value while writing data");
+
 /* Before we attempt to access the TPM we must see that the valid bit is set.
  * The specification says that this bit is 0 at reset and remains 0 until the
  * 'TPM has gone through its self test and initialization and has established
@@ -256,6 +261,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int rc, status, burstcnt;
+	int sendcnt;
 	size_t count = 0;
 	bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
 
@@ -271,19 +277,31 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	}
 
 	while (count < len - 1) {
+
+		/*
+		 * Get the initial burstcount to ensure TPM is ready to
+		 * accept data, even when waiting for burstcount is disabled.
+		 */
 		burstcnt = get_burstcount(chip);
 		if (burstcnt < 0) {
 			dev_err(&chip->dev, "Unable to read burstcount\n");
 			rc = burstcnt;
 			goto out_err;
 		}
-		burstcnt = min_t(int, burstcnt, len - count - 1);
+
+		if (ignore_burst_count)
+			sendcnt = len - 1;
+		else
+			sendcnt = min_t(int, burstcnt, len - count - 1);
+
 		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
-					 burstcnt, buf + count);
+					 sendcnt, buf + count);
 		if (rc < 0)
 			goto out_err;
 
-		count += burstcnt;
+		count += sendcnt;
+		if (ignore_burst_count)
+			continue;
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-- 
2.13.3

[Linux-ima-devel] [PATCH v2 4/4] tpm: use tpm_msleep() value as max delay

[Nayna J. <na...@li...>]

Currently, tpm_msleep() uses delay_msec as the minimum value in
usleep_range. However, that is the maximum time we want to wait.
The function is modified to use the delay_msec as the maximum
value, not the minimum value.

After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~9sec to ~8sec.

Signed-off-by: Nayna Jain <na...@li...>
Acked-by: Mimi Zohar <zo...@li...>
---
 drivers/char/tpm/tpm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index eb2f8818eded..ff5a8b7b80b9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -533,8 +533,8 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 
 static inline void tpm_msleep(unsigned int delay_msec)
 {
-	usleep_range(delay_msec * 1000,
-		     (delay_msec * 1000) + TPM_TIMEOUT_RANGE_US);
+	usleep_range((delay_msec * 1000) - TPM_TIMEOUT_RANGE_US,
+		     delay_msec * 1000);
 };
 
 struct tpm_chip *tpm_chip_find_get(int chip_num);
-- 
2.13.3

[Linux-ima-devel] [PATCH v2 3/4] tpm: reduce tpm_msleep() time in get_burstcount()

[Nayna J. <na...@li...>]

Currently, get_burstcount() function sleeps for 5msec in a loop
before retrying for next query to burstcount. However, if it takes
lesser time for TPM to return, this 5 msec delay is longer
than necessary.

This patch replaces the tpm_msleep time from 5msec to 1msec.

After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~10sec to ~9sec.

Signed-off-by: Nayna Jain <na...@li...>
Acked-by: Mimi Zohar <zo...@li...>
---
 drivers/char/tpm/tpm_tis_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d1eab29cb447..d710bbc4608b 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -169,7 +169,7 @@ static int get_burstcount(struct tpm_chip *chip)
 		burstcnt = (value >> 8) & 0xFFFF;
 		if (burstcnt)
 			return burstcnt;
-		tpm_msleep(TPM_TIMEOUT);
+		tpm_msleep(1);
 	} while (time_before(jiffies, stop));
 	return -EBUSY;
 }
-- 
2.13.3

[Linux-ima-devel] [PATCH v2 2/4] tpm: define __wait_for_tpm_stat to specify variable polling sleep time

[Nayna J. <na...@li...>]

The existing wait_for_tpm_stat() checks the chip status before
sleeping for 5 msec in a polling loop.  For some functions although
the status isn't ready immediately, the status returns extremely
quickly. Waiting for 5 msec causes an unnecessary delay. An
example is the send() call in the tpms_tis driver.

This patch defines __wait_for_tpm_stat(), allowing the caller
to specify the polling sleep timeout value within the loop.
The existing wait_for_tpm_stat() becomes a wrapper for this
function.

After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~14sec to ~10sec.

Signed-off-by: Nayna Jain <na...@li...>
Acked-by: Mimi Zohar <zo...@li...>
---
 drivers/char/tpm/tpm-interface.c | 15 ++++++++++++---
 drivers/char/tpm/tpm.h           |  3 +++
 drivers/char/tpm/tpm_tis_core.c  | 11 ++++++-----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1d6729be4cd6..b23d006243b7 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1050,8 +1050,9 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
 	return false;
 }
 
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
-		      wait_queue_head_t *queue, bool check_cancel)
+int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+		      unsigned int poll_sleep, wait_queue_head_t *queue,
+		      bool check_cancel)
 {
 	unsigned long stop;
 	long rc;
@@ -1085,7 +1086,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 		}
 	} else {
 		do {
-			tpm_msleep(TPM_TIMEOUT);
+			tpm_msleep(poll_sleep);
 			status = chip->ops->status(chip);
 			if ((status & mask) == mask)
 				return 0;
@@ -1093,6 +1094,14 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 	}
 	return -ETIME;
 }
+EXPORT_SYMBOL_GPL(__wait_for_tpm_stat);
+
+int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+		      wait_queue_head_t *queue, bool check_cancel)
+{
+	return __wait_for_tpm_stat(chip, mask, timeout, TPM_TIMEOUT,
+			queue, check_cancel);
+}
 EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
 
 #define TPM_ORD_SAVESTATE 152
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2d5466a72e40..eb2f8818eded 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -525,6 +525,9 @@ int tpm_do_selftest(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
+int __wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+		unsigned long timeout, unsigned int poll_sleep,
+		wait_queue_head_t *queue, bool check_cancel);
 int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 		      wait_queue_head_t *queue, bool check_cancel);
 
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6b9bf4c4d434..d1eab29cb447 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -268,8 +268,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
 		tpm_tis_ready(chip);
-		if (wait_for_tpm_stat
-		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
+		if (__wait_for_tpm_stat
+		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b, 1,
 		     &priv->int_queue, false) < 0) {
 			rc = -ETIME;
 			goto out_err;
@@ -303,7 +303,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 		if (ignore_burst_count)
 			continue;
 
-		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+		if (__wait_for_tpm_stat(chip, TPM_STS_VALID,
+					chip->timeout_c, 1,
 					&priv->int_queue, false) < 0) {
 			rc = -ETIME;
 			goto out_err;
@@ -320,8 +321,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	if (rc < 0)
 		goto out_err;
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
+	if (__wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+				1, &priv->int_queue, false) < 0) {
 		rc = -ETIME;
 		goto out_err;
 	}
-- 
2.13.3

[Linux-ima-devel] [PATCH v2 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Nayna J. <na...@li...>]

The TPM burstcount status indicates the number of bytes that can
be sent to the TPM without causing bus wait states.  Effectively,
it is the number of empty bytes in the command FIFO. Further,
some TPMs have a static burstcount, when the value remains zero
until the entire FIFO is empty.

This patch adds an optimization to check for burstcount only once.
And if it is valid, it writes all the bytes at once, permitting
wait states. The performance of a 34 byte extend on a TPM 1.2 with
an 8 byte burstcount improved from 41 msec to 14 msec.

This functionality is enabled only by passing module
parameter ignore_burst_count=1. By default, this parameter
is disabled.

After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~41sec to ~14sec.

Suggested-by: Ken Goldman <kg...@li...> in
conjunction with the TPM Device Driver work group.
Signed-off-by: Nayna Jain <na...@li...>
Acked-by: Mimi Zohar <zo...@li...>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
 drivers/char/tpm/tpm_tis_core.c                 | 24 +++++++++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4e303be83df6..3c59bb91e1ee 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1465,6 +1465,14 @@
 			mode generally follows that for the NaN encoding,
 			except where unsupported by hardware.
 
+	ignore_burst_count [TPM_TIS_CORE]
+			tpm_tis_core driver queries for the burstcount before
+			every send call in a loop. However, it causes delay to
+			the send command for TPMs with low burstcount value.
+			Setting this value to 1, will make driver to query for
+			burstcount only once in the loop to improve the
+			performance. By default, its value is set to 0.
+
 	ignore_loglevel	[KNL]
 			Ignore loglevel setting - this will print /all/
 			kernel messages to the console. Useful for debugging.
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 63bc6c3b949e..6b9bf4c4d434 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,11 @@
 #include "tpm.h"
 #include "tpm_tis_core.h"
 
+static bool ignore_burst_count = false;
+module_param(ignore_burst_count, bool, 0444);
+MODULE_PARM_DESC(ignore_burst_count,
+		"Ignore burstcount value while writing data");
+
 /* Before we attempt to access the TPM we must see that the valid bit is set.
  * The specification says that this bit is 0 at reset and remains 0 until the
  * 'TPM has gone through its self test and initialization and has established
@@ -256,6 +261,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int rc, status, burstcnt;
+	int sendcnt;
 	size_t count = 0;
 	bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
 
@@ -271,19 +277,31 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	}
 
 	while (count < len - 1) {
+
+		/*
+		 * Get the initial burstcount to ensure TPM is ready to
+		 * accept data, even when waiting for burstcount is disabled.
+		 */
 		burstcnt = get_burstcount(chip);
 		if (burstcnt < 0) {
 			dev_err(&chip->dev, "Unable to read burstcount\n");
 			rc = burstcnt;
 			goto out_err;
 		}
-		burstcnt = min_t(int, burstcnt, len - count - 1);
+
+		if (ignore_burst_count)
+			sendcnt = len - 1;
+		else
+			sendcnt = min_t(int, burstcnt, len - count - 1);
+
 		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
-					 burstcnt, buf + count);
+					 sendcnt, buf + count);
 		if (rc < 0)
 			goto out_err;
 
-		count += burstcnt;
+		count += sendcnt;
+		if (ignore_burst_count)
+			continue;
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-- 
2.13.3

[Linux-ima-devel] [PATCH v2 0/4] additional TPM performance improvements

[Nayna J. <na...@li...>]

After further discussions with the Device Driver working group (ddwg),
the following changes were made: 

* Check for burstcount at least once to confirm the TPM is ready to accept
the data. Similarly, query for the TPM Expect status as sanity check at
the end.

* Make the sleep for status check during send() in the loop less than
5msec.

* Make the sleep in the loop while querying for burstcount less than
5msec.

Below is the list of patches along with the performance improvements
seen with a TPM 1.2 with an 8 byte burstcount for 1000 extends: 

Patch                                        |Improvement(time in sec)

tpm: ignore burstcount to improve tpm_tis    | ~41 - ~14
send() performance.

tpm: define __wait_for_tpm_stat to specify   | ~14 - ~10
variable polling sleep time

tpm: reduce tpm_msleep() time in             | ~10 - ~9
get_burstcount()

tpm: modify tpm_msleep() function to have    | ~9 - ~8
max range

Changelog v2:

* Add module parameter to handle ignoring of burst count during
tpm tis send() operation.
* Add improvements over sleep time to reduce delays.

Nayna Jain (4):
  tpm: ignore burstcount to improve tpm_tis send() performance.
  tpm: define __wait_for_tpm_stat to specify variable polling sleep time
  tpm: reduce tpm_msleep() time in get_burstcount()
  tpm: use tpm_msleep() value as max delay

 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++
 drivers/char/tpm/tpm-interface.c                | 15 ++++++++--
 drivers/char/tpm/tpm.h                          |  7 +++--
 drivers/char/tpm/tpm_tis_core.c                 | 37 +++++++++++++++++++------
 4 files changed, 53 insertions(+), 14 deletions(-)

-- 
2.13.3

[Linux-ima-devel] [PATCH v8 20/20] crypto: adapt api sample to use async. op wait

[Gilad Ben-Y. <gi...@be...>]

The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-    struct tcrypt_result {
-        struct completion completion;
-        int err;
-    };
-
     /* tie all data structures together */
     struct skcipher_def {
         struct scatterlist sg;
         struct crypto_skcipher *tfm;
         struct skcipher_request *req;
-        struct tcrypt_result result;
+        struct crypto_wait wait;
     };
 
-    /* Callback function */
-    static void test_skcipher_cb(struct crypto_async_request *req, int error)
-    {
-        struct tcrypt_result *result = req->data;
-
-        if (error == -EINPROGRESS)
-            return;
-        result->err = error;
-        complete(&result->completion);
-        pr_info("Encryption finished successfully\n");
-    }
-
     /* Perform cipher operation */
     static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
                          int enc)
     {
-        int rc = 0;
+        int rc;
 
         if (enc)
-            rc = crypto_skcipher_encrypt(sk->req);
+            rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
         else
-            rc = crypto_skcipher_decrypt(sk->req);
-
-        switch (rc) {
-        case 0:
-            break;
-        case -EINPROGRESS:
-        case -EBUSY:
-            rc = wait_for_completion_interruptible(
-                &sk->result.completion);
-            if (!rc && !sk->result.err) {
-                reinit_completion(&sk->result.completion);
-                break;
-            }
-        default:
-            pr_info("skcipher encrypt returned with %d result %d\n",
-                rc, sk->result.err);
-            break;
-        }
-        init_completion(&sk->result.completion);
+            rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+	if (rc)
+		pr_info("skcipher encrypt returned with result %d\n", rc);
 
         return rc;
     }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
         }
 
         skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-                          test_skcipher_cb,
-                          &sk.result);
+                          crypto_req_done,
+                          &sk.wait);
 
         /* AES 256 with random key */
         get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
         /* We encrypt one block */
         sg_init_one(&sk.sg, scratchpad, 16);
         skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
-        init_completion(&sk.result.completion);
+        crypto_init_wait(&sk.wait);
 
         /* encrypt data */
         ret = test_skcipher_encdec(&sk, 1);
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 19/20] crypto: mediatek: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Ryder Lee <ryd...@me...>
---
 drivers/crypto/mediatek/mtk-aes.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
 	struct crypto_skcipher *ctr;
 };
 
-struct mtk_aes_gcm_setkey_result {
-	int err;
-	struct completion completion;
-};
-
 struct mtk_aes_drv {
 	struct list_head dev_list;
 	/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 mode)
 				    &req->base);
 }
 
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
-	struct mtk_aes_gcm_setkey_result *result = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	result->err = err;
-	complete(&result->completion);
-}
-
 /*
  * Because of the hardware limitation, we need to pre-calculate key(H)
  * for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 		u32 hash[4];
 		u8 iv[8];
 
-		struct mtk_aes_gcm_setkey_result result;
+		struct crypto_wait wait;
 
 		struct scatterlist sg[1];
 		struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 	if (!data)
 		return -ENOMEM;
 
-	init_completion(&data->result.completion);
+	crypto_init_wait(&data->wait);
 	sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
 	skcipher_request_set_tfm(&data->req, ctr);
 	skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
 				      CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      mtk_gcm_setkey_done, &data->result);
+				      crypto_req_done, &data->wait);
 	skcipher_request_set_crypt(&data->req, data->sg, data->sg,
 				   AES_BLOCK_SIZE, data->iv);
 
-	err = crypto_skcipher_encrypt(&data->req);
-	if (err == -EINPROGRESS || err == -EBUSY) {
-		err = wait_for_completion_interruptible(
-			&data->result.completion);
-		if (!err)
-			err = data->result.err;
-	}
+	err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+			      &data->wait);
 	if (err)
 		goto out;
 
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 18/20] crypto: qce: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The qce driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 drivers/crypto/qce/sha.c | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 47e114a..53227d7 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -349,28 +349,12 @@ static int qce_ahash_digest(struct ahash_request *req)
 	return qce->async_req_enqueue(tmpl->qce, &req->base);
 }
 
-struct qce_ahash_result {
-	struct completion completion;
-	int error;
-};
-
-static void qce_digest_complete(struct crypto_async_request *req, int error)
-{
-	struct qce_ahash_result *result = req->data;
-
-	if (error == -EINPROGRESS)
-		return;
-
-	result->error = error;
-	complete(&result->completion);
-}
-
 static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 				 unsigned int keylen)
 {
 	unsigned int digestsize = crypto_ahash_digestsize(tfm);
 	struct qce_sha_ctx *ctx = crypto_tfm_ctx(&tfm->base);
-	struct qce_ahash_result result;
+	struct crypto_wait wait;
 	struct ahash_request *req;
 	struct scatterlist sg;
 	unsigned int blocksize;
@@ -405,9 +389,9 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 		goto err_free_ahash;
 	}
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   qce_digest_complete, &result);
+				   crypto_req_done, &wait);
 	crypto_ahash_clear_flags(ahash_tfm, ~0);
 
 	buf = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, GFP_KERNEL);
@@ -420,13 +404,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 	sg_init_one(&sg, buf, keylen);
 	ahash_request_set_crypt(req, &sg, ctx->authkey, keylen);
 
-	ret = crypto_ahash_digest(req);
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		ret = wait_for_completion_interruptible(&result.completion);
-		if (!ret)
-			ret = result.error;
-	}
-
+	ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
 	if (ret)
 		crypto_ahash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
 
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 17/20] crypto: talitos: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The talitos driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 drivers/crypto/talitos.c | 38 +++++---------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 79791c6..194a307 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2037,22 +2037,6 @@ static int ahash_import(struct ahash_request *areq, const void *in)
 	return 0;
 }
 
-struct keyhash_result {
-	struct completion completion;
-	int err;
-};
-
-static void keyhash_complete(struct crypto_async_request *req, int err)
-{
-	struct keyhash_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 		   u8 *hash)
 {
@@ -2060,10 +2044,10 @@ static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 
 	struct scatterlist sg[1];
 	struct ahash_request *req;
-	struct keyhash_result hresult;
+	struct crypto_wait wait;
 	int ret;
 
-	init_completion(&hresult.completion);
+	crypto_init_wait(&wait);
 
 	req = ahash_request_alloc(tfm, GFP_KERNEL);
 	if (!req)
@@ -2072,25 +2056,13 @@ static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 	/* Keep tfm keylen == 0 during hash of the long key */
 	ctx->keylen = 0;
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   keyhash_complete, &hresult);
+				   crypto_req_done, &wait);
 
 	sg_init_one(&sg[0], key, keylen);
 
 	ahash_request_set_crypt(req, sg, hash, keylen);
-	ret = crypto_ahash_digest(req);
-	switch (ret) {
-	case 0:
-		break;
-	case -EINPROGRESS:
-	case -EBUSY:
-		ret = wait_for_completion_interruptible(
-			&hresult.completion);
-		if (!ret)
-			ret = hresult.err;
-		break;
-	default:
-		break;
-	}
+	ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+
 	ahash_request_free(req);
 
 	return ret;
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 16/20] crypto: tcrypt: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

tcrypt starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 crypto/tcrypt.c | 84 +++++++++++++++++----------------------------------------
 1 file changed, 25 insertions(+), 59 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 0022a18..802aa81 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -79,34 +79,11 @@ static char *check[] = {
 	NULL
 };
 
-struct tcrypt_result {
-	struct completion completion;
-	int err;
-};
-
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
-	struct tcrypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static inline int do_one_aead_op(struct aead_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
+	struct crypto_wait *wait = req->base.data;
 
-		ret = wait_for_completion_interruptible(&tr->completion);
-		if (!ret)
-			ret = tr->err;
-		reinit_completion(&tr->completion);
-	}
-
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 static int test_aead_jiffies(struct aead_request *req, int enc,
@@ -248,7 +225,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 	char *axbuf[XBUFSIZE];
 	unsigned int *b_size;
 	unsigned int iv_len;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 
 	iv = kzalloc(MAX_IVLEN, GFP_KERNEL);
 	if (!iv)
@@ -284,7 +261,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 		goto out_notfm;
 	}
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 	printk(KERN_INFO "\ntesting speed of %s (%s) %s\n", algo,
 			get_driver_name(crypto_aead, tfm), e);
 
@@ -296,7 +273,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 	}
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  tcrypt_complete, &result);
+				  crypto_req_done, &wait);
 
 	i = 0;
 	do {
@@ -397,21 +374,16 @@ static void test_hash_sg_init(struct scatterlist *sg)
 
 static inline int do_one_ahash_op(struct ahash_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
+	struct crypto_wait *wait = req->base.data;
 
-		wait_for_completion(&tr->completion);
-		reinit_completion(&tr->completion);
-		ret = tr->err;
-	}
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 struct test_mb_ahash_data {
 	struct scatterlist sg[TVMEMSIZE];
 	char result[64];
 	struct ahash_request *req;
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	char *xbuf[XBUFSIZE];
 };
 
@@ -440,7 +412,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 		if (testmgr_alloc_buf(data[i].xbuf))
 			goto out;
 
-		init_completion(&data[i].tresult.completion);
+		crypto_init_wait(&data[i].wait);
 
 		data[i].req = ahash_request_alloc(tfm, GFP_KERNEL);
 		if (!data[i].req) {
@@ -449,8 +421,8 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 			goto out;
 		}
 
-		ahash_request_set_callback(data[i].req, 0,
-					   tcrypt_complete, &data[i].tresult);
+		ahash_request_set_callback(data[i].req, 0, crypto_req_done,
+					   &data[i].wait);
 		test_hash_sg_init(data[i].sg);
 	}
 
@@ -492,16 +464,16 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 			if (ret)
 				break;
 
-			complete(&data[k].tresult.completion);
-			data[k].tresult.err = 0;
+			crypto_req_done(&data[k].req->base, 0);
 		}
 
 		for (j = 0; j < k; j++) {
-			struct tcrypt_result *tr = &data[j].tresult;
+			struct crypto_wait *wait = &data[j].wait;
+			int wait_ret;
 
-			wait_for_completion(&tr->completion);
-			if (tr->err)
-				ret = tr->err;
+			wait_ret = crypto_wait_req(-EINPROGRESS, wait);
+			if (wait_ret)
+				ret = wait_ret;
 		}
 
 		end = get_cycles();
@@ -679,7 +651,7 @@ static void test_ahash_speed_common(const char *algo, unsigned int secs,
 				    struct hash_speed *speed, unsigned mask)
 {
 	struct scatterlist sg[TVMEMSIZE];
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	struct ahash_request *req;
 	struct crypto_ahash *tfm;
 	char *output;
@@ -708,9 +680,9 @@ static void test_ahash_speed_common(const char *algo, unsigned int secs,
 		goto out;
 	}
 
-	init_completion(&tresult.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   tcrypt_complete, &tresult);
+				   crypto_req_done, &wait);
 
 	output = kmalloc(MAX_DIGEST_SIZE, GFP_KERNEL);
 	if (!output)
@@ -765,15 +737,9 @@ static void test_hash_speed(const char *algo, unsigned int secs,
 
 static inline int do_one_acipher_op(struct skcipher_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
-
-		wait_for_completion(&tr->completion);
-		reinit_completion(&tr->completion);
-		ret = tr->err;
-	}
+	struct crypto_wait *wait = req->base.data;
 
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 static int test_acipher_jiffies(struct skcipher_request *req, int enc,
@@ -853,7 +819,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 				unsigned int tcount, u8 *keysize, bool async)
 {
 	unsigned int ret, i, j, k, iv_len;
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	const char *key;
 	char iv[128];
 	struct skcipher_request *req;
@@ -866,7 +832,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 	else
 		e = "decryption";
 
-	init_completion(&tresult.completion);
+	crypto_init_wait(&wait);
 
 	tfm = crypto_alloc_skcipher(algo, 0, async ? 0 : CRYPTO_ALG_ASYNC);
 
@@ -887,7 +853,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 	}
 
 	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      tcrypt_complete, &tresult);
+				      crypto_req_done, &wait);
 
 	i = 0;
 	do {
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 15/20] ima: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

ima starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_crypto.c | 56 +++++++++++--------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index a856d8c..9057b16 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
 
 #include "ima.h"
 
-struct ahash_completion {
-	struct completion completion;
-	int err;
-};
-
 /* minimum file size for ahash use */
 static unsigned long ima_ahash_minsize;
 module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
 		crypto_free_ahash(tfm);
 }
 
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
 {
-	struct ahash_completion *res = req->data;
 
-	if (err == -EINPROGRESS)
-		return;
-	res->err = err;
-	complete(&res->completion);
-}
+	err = crypto_wait_req(err, wait);
 
-static int ahash_wait(int err, struct ahash_completion *res)
-{
-	switch (err) {
-	case 0:
-		break;
-	case -EINPROGRESS:
-	case -EBUSY:
-		wait_for_completion(&res->completion);
-		reinit_completion(&res->completion);
-		err = res->err;
-		/* fall through */
-	default:
+	if (err)
 		pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
-	}
 
 	return err;
 }
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
 	struct ahash_request *req;
 	struct scatterlist sg[1];
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	size_t rbuf_size[2];
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out1;
 
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			active = !active; /* swap buffers, if we use two */
 	}
 	/* wait for the last update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 out3:
 	if (read)
 		file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 out2:
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out1:
 	ahash_request_free(req);
@@ -537,7 +515,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 {
 	struct ahash_request *req;
 	struct scatterlist sg;
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	int rc, ahash_rc = 0;
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -546,12 +524,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out;
 
@@ -561,10 +539,10 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	ahash_rc = crypto_ahash_update(req);
 
 	/* wait for the update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out:
 	ahash_request_free(req);
-- 
2.1.4

[Linux-ima-devel] [PATCH v8 14/20] cifs: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Pavel Shilovsky <ps...@mi...>
---
 fs/cifs/smb2ops.c | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fb2934b..982b39d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2066,22 +2066,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
 	return sg;
 }
 
-struct cifs_crypt_result {
-	int err;
-	struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
-	struct cifs_crypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int
 smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
 {
@@ -2122,12 +2106,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	struct aead_request *req;
 	char *iv;
 	unsigned int iv_len;
-	struct cifs_crypt_result result = {0, };
+	DECLARE_CRYPTO_WAIT(wait);
 	struct crypto_aead *tfm;
 	unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
 
-	init_completion(&result.completion);
-
 	rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
 	if (rc) {
 		cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -2187,14 +2169,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	aead_request_set_ad(req, assoc_data_len);
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  cifs_crypt_complete, &result);
+				  crypto_req_done, &wait);
 
-	rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
-	if (rc == -EINPROGRESS || rc == -EBUSY) {
-		wait_for_completion(&result.completion);
-		rc = result.err;
-	}
+	rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+				: crypto_aead_decrypt(req), &wait);
 
 	if (!rc && enc)
 		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
-- 
2.1.4