On Fri, Feb 05, 2021 at 03:55:09PM +0100, Lino Sanfilippo wrote:
Hi,
On 05.02.21 14:05, Jason Gunthorpe wrote:
Commit fdc915f7f719 ("tpm: expose spaces via a device link /dev/tpmrm<n>") already introduced function tpm_devs_release() to release the extra reference but did not implement the required put on chip->devs that results in the call of this function.
Seems wonky, the devs is just supposed to be a side thing, nothing should be using it as a primary reference count for a tpm.
The bug here is only that tpm_common_open() did not get a kref on the chip before putting it in priv and linking it to the fd. See the comment before tpm_try_get_ops() indicating the caller must already have taken care to ensure the chip is valid.
This should be all you need to fix the oops:
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c index 1784530b8387bb..1b738dca7fffb5 100644 +++ b/drivers/char/tpm/tpm-dev-common.c @@ -105,6 +105,7 @@ static void tpm_timeout_work(struct work_struct *work) void tpm_common_open(struct file *file, struct tpm_chip *chip, struct file_priv *priv, struct tpm_space *space) {
get_device(&priv->chip.dev); priv->chip = chip; priv->space = space; priv->response_read = true;This is racy, isnt it? The time between we open the file and we want to grab the reference in common_open() the chip can already be unregistered and freed.
No, the cdev layer holds the refcount on the device while open is being called.
Jason