Skip to content

Commit

Permalink
tpm: fix reference counting for tpmrm device
Browse files Browse the repository at this point in the history
The tpm core module creates two device files (/dev/tpm* and /dev/tpmrm*)
as soon as a tpm chip registers.
However it is possible that the tpm chip unregisters while one or both of
these files are still opened for read/write operations. In case that only
the tpmrm file is still open after chip registration and the file is
accessed for a write operation the following warning is triggered:

 WARNING: CPU: 2 PID: 8800 at lib/refcount.c:153 refcount_inc_checked+0x4c/0x50
 refcount_t: increment on 0; use-after-free.
 Modules linked in: tpm_tis_spi tpm_tis_core tpm xt_nat veth ipt_MASQUERADE nf_conntrack_netlink nfnetlink xt_addrtype iptable_nat nf_nat_ipv4 nf_nat br_netfilter bridge stp llc overlay cmac bnep hci_uart btbcm serdev bluetooth ecdh_generic brcmfmac evdev brcmutil sha256_generic ftdi_sio usbserial cfg80211 bcm2835_codec(C) raspberrypi_hwmon bcm2835_v4l2(C) hwmon v4l2_mem2mem bcm2835_mmal_vchiq(C) v4l2_common videobuf2_dma_contig videobuf2_vmalloc rfkill videobuf2_memops videobuf2_v4l2 videobuf2_common videodev ip6t_REJECT vc_sm_cma(C) nf_reject_ipv6 media ip6t_rt ip6table_filter ip6_tables gpio_wdt gpio_keys fixed mcp320x ad5446 industrialio uio_pdrv_genirq spidev uio ipt_REJECT nf_reject_ipv4 xt_recent xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_limit iptable_filter
  i2c_dev ip_tables x_tables ipv6 [last unloaded: spi_bcm2835]
 CPU: 2 PID: 8800 Comm: iotedged Tainted: G         C        4.19.95-rt38-LS-v7+ RevolutionPi#1
 Hardware name: BCM2835
 [<80112a08>] (unwind_backtrace) from [<8010d5fc>] (show_stack+0x20/0x24)
 [<8010d5fc>] (show_stack) from [<808b3b50>] (dump_stack+0xd4/0x118)
 [<808b3b50>] (dump_stack) from [<80121edc>] (__warn.part.0+0xcc/0xe8)
 [<80121edc>] (__warn.part.0) from [<80121f74>] (warn_slowpath_fmt+0x7c/0xa4)
 [<80121f74>] (warn_slowpath_fmt) from [<805670f8>] (refcount_inc_checked+0x4c/0x50)
 [<805670f8>] (refcount_inc_checked) from [<808b8924>] (kobject_get+0x2c/0x5c)
 [<808b8924>] (kobject_get) from [<8060f57c>] (get_device+0x24/0x2c)
 [<8060f57c>] (get_device) from [<7f1b4384>] (tpm_try_get_ops+0x24/0x58 [tpm])
 [<7f1b4384>] (tpm_try_get_ops [tpm]) from [<7f1b6dbc>] (tpm_common_write+0x17c/0x1fc [tpm])
 [<7f1b6dbc>] (tpm_common_write [tpm]) from [<7f1b89a4>] (tpmrm_write+0x58/0x6b4 [tpm])
 [<7f1b89a4>] (tpmrm_write [tpm]) from [<802ee854>] (__vfs_write+0x4c/0x17c)
 [<802ee854>] (__vfs_write) from [<802eeb6c>] (vfs_write+0xb4/0x1c8)
 [<802eeb6c>] (vfs_write) from [<802eee5c>] (ksys_write+0x70/0xfc)
 [<802eee5c>] (ksys_write) from [<802eef00>] (sys_write+0x18/0x1c)
 [<802eef00>] (sys_write) from [<80101000>] (ret_fast_syscall+0x0/0x28)
 Exception stack(0x9ac97fa8 to 0x9ac97ff0)
 7fa0:                   0000000e 76ef506c 00000009 76ef506c 0000000e 00000000
 7fc0: 0000000e 76ef506c 00000009 00000004 76ef506c 0000000e 00000000 00000001
 7fe0: 00000004 7e93e110 76c54371 76c56526
 ---[ end trace 0000000000000002 ]---
 ------------[ cut here ]------------
 WARNING: CPU: 2 PID: 8800 at lib/refcount.c:187 refcount_sub_and_test_checked+0xb0/0xb8
 refcount_t: underflow; use-after-free.
 Modules linked in: tpm_tis_spi tpm_tis_core tpm xt_nat veth ipt_MASQUERADE nf_conntrack_netlink nfnetlink xt_addrtype iptable_nat nf_nat_ipv4 nf_nat br_netfilter bridge stp llc overlay cmac bnep hci_uart btbcm serdev bluetooth ecdh_generic brcmfmac evdev brcmutil sha256_generic ftdi_sio usbserial cfg80211 bcm2835_codec(C) raspberrypi_hwmon bcm2835_v4l2(C) hwmon v4l2_mem2mem bcm2835_mmal_vchiq(C) v4l2_common videobuf2_dma_contig videobuf2_vmalloc rfkill videobuf2_memops videobuf2_v4l2 videobuf2_common videodev ip6t_REJECT vc_sm_cma(C) nf_reject_ipv6 media ip6t_rt ip6table_filter ip6_tables gpio_wdt gpio_keys fixed mcp320x ad5446 industrialio uio_pdrv_genirq spidev uio ipt_REJECT nf_reject_ipv4 xt_recent xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_limit iptable_filter
  i2c_dev ip_tables x_tables ipv6 [last unloaded: spi_bcm2835]
 CPU: 2 PID: 8800 Comm: iotedged Tainted: G        WC        4.19.95-rt38-LS-v7+ RevolutionPi#1
 Hardware name: BCM2835
 [<80112a08>] (unwind_backtrace) from [<8010d5fc>] (show_stack+0x20/0x24)
 [<8010d5fc>] (show_stack) from [<808b3b50>] (dump_stack+0xd4/0x118)
 [<808b3b50>] (dump_stack) from [<80121edc>] (__warn.part.0+0xcc/0xe8)
 [<80121edc>] (__warn.part.0) from [<80121f74>] (warn_slowpath_fmt+0x7c/0xa4)
 [<80121f74>] (warn_slowpath_fmt) from [<805671ac>] (refcount_sub_and_test_checked+0xb0/0xb8)
 [<805671ac>] (refcount_sub_and_test_checked) from [<805671cc>] (refcount_dec_and_test_checked+0x18/0x1c)
 [<805671cc>] (refcount_dec_and_test_checked) from [<808b8aa8>] (kobject_put+0x2c/0xe0)
 [<808b8aa8>] (kobject_put) from [<8060f788>] (put_device+0x24/0x28)
 [<8060f788>] (put_device) from [<7f1b43b0>] (tpm_try_get_ops+0x50/0x58 [tpm])
 [<7f1b43b0>] (tpm_try_get_ops [tpm]) from [<7f1b6dbc>] (tpm_common_write+0x17c/0x1fc [tpm])
 [<7f1b6dbc>] (tpm_common_write [tpm]) from [<7f1b89a4>] (tpmrm_write+0x58/0x6b4 [tpm])
 [<7f1b89a4>] (tpmrm_write [tpm]) from [<802ee854>] (__vfs_write+0x4c/0x17c)
 [<802ee854>] (__vfs_write) from [<802eeb6c>] (vfs_write+0xb4/0x1c8)
 [<802eeb6c>] (vfs_write) from [<802eee5c>] (ksys_write+0x70/0xfc)
 [<802eee5c>] (ksys_write) from [<802eef00>] (sys_write+0x18/0x1c)
 [<802eef00>] (sys_write) from [<80101000>] (ret_fast_syscall+0x0/0x28)
 Exception stack(0x9ac97fa8 to 0x9ac97ff0)
 7fa0:                   0000000e 76ef506c 00000009 76ef506c 0000000e 00000000
 7fc0: 0000000e 76ef506c 00000009 00000004 76ef506c 0000000e 00000000 00000001
 7fe0: 00000004 7e93e110 76c54371 76c56526
 ---[ end trace 0000000000000003 ]---

The reason is an errorneous reference counting for the tpm dev: In
tpm_chip_alloc() an extra reference ought to be retrieved in case that the
tpmrm file is still open while the tpm file is closed. However getting the
reference depends on the flag TPM_CHIP_FLAG_TPM2, which is never set in the
source code (the original intention was obviously to set it in case that
tpm2 and thus the tpmrm file should be used).

Missing to get this extra reference causes the warning in the error case
described above.

Fix this by getting an extra reference to the tmp dev in tpm_common_open()
which is called for both the tpm and the tpmrm device. This makes sure that
the tpmrm device still holds a valid reference even if the tpm device is
already closed.
Put the respective reference in tpm_common_release() which are also called
for both devices and thus ensures that all references are released
eventually.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
  • Loading branch information
linosanfilippo-kunbus committed Jan 8, 2021
1 parent e7dd750 commit 6b6e63b
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 6 deletions.
6 changes: 2 additions & 4 deletions drivers/char/tpm/tpm-chip.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,13 @@ static void tpm_dev_release(struct device *dev)
kfree(chip->log.bios_event_log);
kfree(chip->work_space.context_buf);
kfree(chip->work_space.session_buf);
put_device(&chip->devs);
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);
/* nothing */
}

/**
Expand Down
4 changes: 4 additions & 0 deletions drivers/char/tpm/tpm-dev-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void tpm_common_open(struct file *file, struct tpm_chip *chip,
struct file_priv *priv)
{
priv->chip = chip;
get_device(&chip->dev);
mutex_init(&priv->buffer_mutex);
timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
INIT_WORK(&priv->work, timeout_work);
Expand Down Expand Up @@ -142,8 +143,11 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
*/
void tpm_common_release(struct file *file, struct file_priv *priv)
{
struct tpm_chip *chip = priv->chip;

del_singleshot_timer_sync(&priv->user_read_timer);
flush_work(&priv->work);
file->private_data = NULL;
priv->data_pending = 0;
put_device(&chip->dev);
}
2 changes: 1 addition & 1 deletion drivers/char/tpm/tpm-dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ static int tpm_release(struct inode *inode, struct file *file)
{
struct file_priv *priv = file->private_data;

tpm_common_release(file, priv);
clear_bit(0, &priv->chip->is_open);
tpm_common_release(file, priv);
kfree(priv);

return 0;
Expand Down
2 changes: 1 addition & 1 deletion drivers/char/tpm/tpmrm-dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ static int tpmrm_release(struct inode *inode, struct file *file)
struct file_priv *fpriv = file->private_data;
struct tpmrm_priv *priv = container_of(fpriv, struct tpmrm_priv, priv);

tpm_common_release(file, fpriv);
tpm2_del_space(fpriv->chip, &priv->space);
tpm_common_release(file, fpriv);
kfree(priv);

return 0;
Expand Down

0 comments on commit 6b6e63b

Please sign in to comment.