On Thu, Jun 21, 2018 at 04:59:55PM -0400, Stefan Berger wrote: > On 06/21/2018 04:53 PM, Mimi Zohar wrote: > >On Wed, 2018-06-20 at 16:42 -0400, Stefan Berger wrote: > >>Rather than accessing the TPM functions using a NULL pointer, which > >>causes a lookup for a suitable chip every time, get a hold of a tpm_chip > >>and access the TPM functions using this chip. We call the tpm_chip > >>ima_tpm_chip and protect it, once initialization is done, using a > >>rw_semaphore called ima_tpm_chip_lock. > >> > >>Use ima_shutdown to release the tpm_chip. > >> > >>Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx> > >> security/integrity/ima/ima.h | 3 +++ > >> security/integrity/ima/ima_crypto.c | 12 ++++++++++-- > >> security/integrity/ima/ima_init.c | 19 ++++++++++++------- > >> security/integrity/ima/ima_queue.c | 7 +++++-- > >> 4 files changed, 30 insertions(+), 11 deletions(-) > >> > >>diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > >>index 354bb5716ce3..53a88d578ca5 100644 > >>+++ b/security/integrity/ima/ima.h > >>@@ -24,6 +24,7 @@ > >> #include <linux/hash.h> > >> #include <linux/tpm.h> > >> #include <linux/audit.h> > >>+#include <linux/rwsem.h> > >> #include <crypto/hash_info.h> > >> > >> #include "../integrity.h" > >>@@ -56,6 +57,8 @@ extern int ima_policy_flag; > >> extern int ima_used_chip; > >> extern int ima_hash_algo; > >> extern int ima_appraise; > >>+extern struct rw_semaphore ima_tpm_chip_lock; > >>+extern struct tpm_chip *ima_tpm_chip; > > > >ima_add_templatE_entry() synchronizes appending a measurement to the > >measurement list and extending the TPM by taking a lock. Do we really > >need to introduce another lock? > > This lock protects the ima_tpm_chip from going from != NULL to NULL in the > ima_shutdown function. Basically, a global pointer accessed by concurrent > threads should be protected if its value can change. However, in this case > ima_shutdown would be called so late that there shouldn't be concurrency > anymore. Though, I found it better to protect it. Maybe someone else has an > opinion? Why have a shutdown block? There is no harm in holding a kref if the machine is shutting down. Jason
