Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cb64bd03 authored by Lino Sanfilippo's avatar Lino Sanfilippo Committed by Greg Kroah-Hartman
Browse files

tpm: fix reference counting for struct tpm_chip 



commit 7e0438f83dc769465ee663bb5dcf8cc154940712 upstream.

The following sequence of operations results in a refcount warning:

1. Open device /dev/tpmrm.
2. Remove module tpm_tis_spi.
3. Write a TPM command to the file descriptor opened at step 1.

------------[ cut here ]------------
WARNING: CPU: 3 PID: 1161 at lib/refcount.c:25 kobject_get+0xa0/0xa4
refcount_t: addition on 0; use-after-free.
Modules linked in: tpm_tis_spi tpm_tis_core tpm mdio_bcm_unimac brcmfmac
sha256_generic libsha256 sha256_arm hci_uart btbcm bluetooth cfg80211 vc4
brcmutil ecdh_generic ecc snd_soc_core crc32_arm_ce libaes
raspberrypi_hwmon ac97_bus snd_pcm_dmaengine bcm2711_thermal snd_pcm
snd_timer genet snd phy_generic soundcore [last unloaded: spi_bcm2835]
CPU: 3 PID: 1161 Comm: hold_open Not tainted 5.10.0ls-main-dirty #2
Hardware name: BCM2711
[<c0410c3c>] (unwind_backtrace) from [<c040b580>] (show_stack+0x10/0x14)
[<c040b580>] (show_stack) from [<c1092174>] (dump_stack+0xc4/0xd8)
[<c1092174>] (dump_stack) from [<c0445a30>] (__warn+0x104/0x108)
[<c0445a30>] (__warn) from [<c0445aa8>] (warn_slowpath_fmt+0x74/0xb8)
[<c0445aa8>] (warn_slowpath_fmt) from [<c08435d0>] (kobject_get+0xa0/0xa4)
[<c08435d0>] (kobject_get) from [<bf0a715c>] (tpm_try_get_ops+0x14/0x54 [tpm])
[<bf0a715c>] (tpm_try_get_ops [tpm]) from [<bf0a7d6c>] (tpm_common_write+0x38/0x60 [tpm])
[<bf0a7d6c>] (tpm_common_write [tpm]) from [<c05a7ac0>] (vfs_write+0xc4/0x3c0)
[<c05a7ac0>] (vfs_write) from [<c05a7ee4>] (ksys_write+0x58/0xcc)
[<c05a7ee4>] (ksys_write) from [<c04001a0>] (ret_fast_syscall+0x0/0x4c)
Exception stack(0xc226bfa8 to 0xc226bff0)
bfa0:                   00000000 000105b4 00000003 beafe664 00000014 00000000
bfc0: 00000000 000105b4 000103f8 00000004 00000000 00000000 b6f9c000 beafe684
bfe0: 0000006c beafe648 0001056c b6eb6944
---[ end trace d4b8409def9b8b1f ]---

The reason for this warning is the attempt to get the chip->dev reference
in tpm_common_write() although the reference counter is already zero.

Since commit 8979b02a ("tpm: Fix reference count to main device") the
extra reference used to prevent a premature zero counter is never taken,
because the required TPM_CHIP_FLAG_TPM2 flag is never set.

Fix this by moving the TPM 2 character device handling from
tpm_chip_alloc() to tpm_add_char_device() which is called at a later point
in time when the flag has been set in case of TPM2.

Commit fdc915f7 ("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.

Fix this by putting chip->devs in tpm_chip_unregister().

Finally move the new implementation for the TPM 2 handling into a new
function to avoid multiple checks for the TPM_CHIP_FLAG_TPM2 flag in the
good case and error cases.

Cc: stable@vger.kernel.org
Fixes: fdc915f7 ("tpm: expose spaces via a device link /dev/tpmrm<n>")
Fixes: 8979b02a ("tpm: Fix reference count to main device")
Co-developed-by: default avatarJason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: default avatarJason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: default avatarLino Sanfilippo <LinoSanfilippo@gmx.de>
Tested-by: default avatarStefan Berger <stefanb@linux.ibm.com>
Reviewed-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 99db2821

drivers/char/tpm/tpm-chip.c

+8 −38
Original line number Diff line number Diff line
@@ -163,14 +163,6 @@ static void tpm_dev_release(struct device *dev) 
	kfree(chip);

}



static void tpm_devs_release(struct device *dev)

{

	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devs);



	/* release the master device reference */

	put_device(&chip->dev);

}



/**

 * tpm_class_shutdown() - prepare the TPM device for loss of power.

 * @dev: device to which the chip is associated.
@@ -234,7 +226,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, 
	chip->dev_num = rc;



	device_initialize(&chip->dev);

	device_initialize(&chip->devs);



	chip->dev.class = tpm_class;

	chip->dev.class->shutdown_pre = tpm_class_shutdown;
@@ -242,29 +233,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, 
	chip->dev.parent = pdev;

	chip->dev.groups = chip->groups;



	chip->devs.parent = pdev;

	chip->devs.class = tpmrm_class;

	chip->devs.release = tpm_devs_release;

	/* get extra reference on main device to hold on

	 * behalf of devs.  This holds the chip structure

	 * while cdevs is in use.  The corresponding put

	 * is in the tpm_devs_release (TPM2 only)

	 */

	if (chip->flags & TPM_CHIP_FLAG_TPM2)

		get_device(&chip->dev);



	if (chip->dev_num == 0)

		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);

	else

		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);



	chip->devs.devt =

		MKDEV(MAJOR(tpm_devt), chip->dev_num + TPM_NUM_DEVICES);



	rc = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);

	if (rc)

		goto out;

	rc = dev_set_name(&chip->devs, "tpmrm%d", chip->dev_num);

	if (rc)

		goto out;
@@ -272,9 +246,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, 
		chip->flags |= TPM_CHIP_FLAG_VIRTUAL;



	cdev_init(&chip->cdev, &tpm_fops);

	cdev_init(&chip->cdevs, &tpmrm_fops);

	chip->cdev.owner = THIS_MODULE;

	chip->cdevs.owner = THIS_MODULE;



	rc = tpm2_init_space(&chip->work_space, TPM2_SPACE_BUFFER_SIZE);

	if (rc) {
@@ -286,7 +258,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, 
	return chip;



out:

	put_device(&chip->devs);

	put_device(&chip->dev);

	return ERR_PTR(rc);

}
@@ -335,14 +306,9 @@ static int tpm_add_char_device(struct tpm_chip *chip) 
	}



	if (chip->flags & TPM_CHIP_FLAG_TPM2) {

		rc = cdev_device_add(&chip->cdevs, &chip->devs);

		if (rc) {

			dev_err(&chip->devs,

				"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",

				dev_name(&chip->devs), MAJOR(chip->devs.devt),

				MINOR(chip->devs.devt), rc);

			return rc;

		}

		rc = tpm_devs_add(chip);

		if (rc)

			goto err_del_cdev;

	}



	/* Make the chip available. */
@@ -350,6 +316,10 @@ static int tpm_add_char_device(struct tpm_chip *chip) 
	idr_replace(&dev_nums_idr, chip, chip->dev_num);

	mutex_unlock(&idr_lock);



	return 0;



err_del_cdev:

	cdev_device_del(&chip->cdev, &chip->dev);

	return rc;

}
@@ -508,7 +478,7 @@ void tpm_chip_unregister(struct tpm_chip *chip) 
		hwrng_unregister(&chip->hwrng);

	tpm_bios_log_teardown(chip);

	if (chip->flags & TPM_CHIP_FLAG_TPM2)

		cdev_device_del(&chip->cdevs, &chip->devs);

		tpm_devs_remove(chip);

	tpm_del_char_device(chip);

}

EXPORT_SYMBOL_GPL(tpm_chip_unregister);

drivers/char/tpm/tpm.h

+2 −0
Original line number Diff line number Diff line
@@ -605,6 +605,8 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, 
		       u8 *cmd);

int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,

		      u32 cc, u8 *buf, size_t *bufsiz);

int tpm_devs_add(struct tpm_chip *chip);

void tpm_devs_remove(struct tpm_chip *chip);



void tpm_bios_log_setup(struct tpm_chip *chip);

void tpm_bios_log_teardown(struct tpm_chip *chip);

drivers/char/tpm/tpm2-space.c

+65 −0
Original line number Diff line number Diff line
@@ -536,3 +536,68 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, 


	return 0;

}



/*

 * Put the reference to the main device.

 */

static void tpm_devs_release(struct device *dev)

{

	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devs);



	/* release the master device reference */

	put_device(&chip->dev);

}



/*

 * Remove the device file for exposed TPM spaces and release the device

 * reference. This may also release the reference to the master device.

 */

void tpm_devs_remove(struct tpm_chip *chip)

{

	cdev_device_del(&chip->cdevs, &chip->devs);

	put_device(&chip->devs);

}



/*

 * Add a device file to expose TPM spaces. Also take a reference to the

 * main device.

 */

int tpm_devs_add(struct tpm_chip *chip)

{

	int rc;



	device_initialize(&chip->devs);

	chip->devs.parent = chip->dev.parent;

	chip->devs.class = tpmrm_class;



	/*

	 * Get extra reference on main device to hold on behalf of devs.

	 * This holds the chip structure while cdevs is in use. The

	 * corresponding put is in the tpm_devs_release.

	 */

	get_device(&chip->dev);

	chip->devs.release = tpm_devs_release;

	chip->devs.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num + TPM_NUM_DEVICES);

	cdev_init(&chip->cdevs, &tpmrm_fops);

	chip->cdevs.owner = THIS_MODULE;



	rc = dev_set_name(&chip->devs, "tpmrm%d", chip->dev_num);

	if (rc)

		goto err_put_devs;



	rc = cdev_device_add(&chip->cdevs, &chip->devs);

	if (rc) {

		dev_err(&chip->devs,

			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",

			dev_name(&chip->devs), MAJOR(chip->devs.devt),

			MINOR(chip->devs.devt), rc);

		goto err_put_devs;

	}



	return 0;



err_put_devs:

	put_device(&chip->devs);



	return rc;

}