lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Aug 2018 11:21:14 -0700 From: Tadeusz Struk <tadeusz.struk@...el.com> To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com> Cc: flihp@...bit.us, jgg@...pe.ca, linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v4 2/2] tpm: add support for nonblocking operation On 08/10/2018 10:43 AM, Jarkko Sakkinen wrote: >> +static struct workqueue_struct *tpm_dev_wq; > A naming contradiction with tpm_common_read() and tpm_common_write(). To > sort that up I would suggest adding a commit to the patch set that > renames these functions as tpm_dev_common_read() and > tpm_dev_common_write() and use the name tpm_common_dev_wq here. > Currently we have: tpm_open(), tpm_write(), tpm_release() in tpm-dev.c tpmrm_open(), tpmrm_read(), tpmrm_write(), tpmrm_release() in tpmrm-dev.c tpm_common_open(), tpm_common_read(), tpm_common_write(), tpm_common_release() in tpm-dev-common.c I think that's pretty consistent. Do you want me to rename all of them to tpm_dev_*()? I don't see any value in doing this. What about if I just rename: tpm_dev_wq_lock to tpm_common_wq_lock, and tpm_dev_wq to tpm_common_wq? >> +static DEFINE_MUTEX(tpm_dev_wq_lock); > This is an unacceptable way to do it, Rather add: > > int __init tpm_dev_common_init(void) > { > tpm_dev_common_wq = alloc_workqueue("tpm_dev_wq", WQ_MEM_RECLAIM, 0); > if (!tpm_dev_common_wq) > return -ENOMEM; > > return 0; > } > > and call this in the driver initialization. > That was the way it was implemented in v1 https://patchwork.kernel.org/patch/10442125/ See: static int __init tpm_dev_common_init(void) and the feedback I got from Jason was: "I wonder if it is worth creating this when the first file is opened.. Lots of systems have TPMs but few use the userspace.." so I changed this to allocate the WQ on first open. I think it makes sense, but I leave it to you to decide. Tadeusz, -- Tadeusz
Powered by blists - more mailing lists