From: Stefan Berger <stefanb@linux.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org,
James Bottomley <James.Bottomley@HansenPartnership.com>,
Tomas Winkler <tomas.winkler@intel.com>,
Tadeusz Struk <tadeusz.struk@intel.com>,
Stefan Berger <stefanb@linux.vnet.ibm.com>,
Nayna Jain <nayna@linux.ibm.com>, Peter Huewe <peterhuewe@gmx.de>,
Jason Gunthorpe <jgg@ziepe.ca>, Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 15/17] tpm: introduce tpm_chip_start() and tpm_chip_stop()
Date: Tue, 13 Nov 2018 17:34:54 -0500 [thread overview]
Message-ID: <f13401b7-e683-6b61-b0cf-7c4be87903cb@linux.ibm.com> (raw)
In-Reply-To: <20181113183620.27447-16-jarkko.sakkinen@linux.intel.com>
On 11/13/18 1:36 PM, Jarkko Sakkinen wrote:
> Encapsulate power gating and locality functionality to tpm_chip_start()
> and tpm_chip_stop() in order to clean up the branching mess in
> tpm_transmit().
I ran the vtpm proxy test suite on this series and got this error when
running it with 'make check-j5'
(https://github.com/stefanberger/linux-vtpm-tests).
It's was actually difficult to hit this bug since I ran it with -j1 ,
-j2 and so on before.
[ 3003.838138] BUG: unable to handle kernel NULL pointer dereference at
0000000000000000
[ 3003.838806] PGD 0 P4D 0
[ 3003.838806] Oops: 0010 [#1] SMP PTI
[ 3003.840394] CPU: 3 PID: 111 Comm: kworker/3:1 Not tainted 4.20.0-rc2+ #6
[ 3003.840394] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.0-50-g14221cd-dirty-20181018_164544-sbct-4.pok.ibm.com
04/01/2014
[ 3003.840394] Workqueue: tpm-vtpm vtpm_proxy_work
[ 3003.840394] RIP: 0010: (null)
[ 3003.840394] Code: Bad RIP value.
[ 3003.840394] RSP: 0018:ffffa6e6816d7e10 EFLAGS: 00010246
[ 3003.840394] RAX: 00000000ffffffe0 RBX: ffff9698077ad000 RCX:
0000000000000006
[ 3003.840394] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
ffff9698077ad000
[ 3003.840394] RBP: ffff9698077ad000 R08: fffff59bc6172d08 R09:
0000000000000000
[ 3003.840394] R10: 0000000000000000 R11: 0000000000000000 R12:
ffff9698b78f0700
[ 3003.840394] R13: ffff9698b78fbc00 R14: 0000000000000000 R15:
ffff9697f8ea10f8
[ 3003.840394] FS: 0000000000000000(0000) GS:ffff9698b78c0000(0000)
knlGS:0000000000000000
[ 3003.840394] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3003.840394] CR2: ffffffffffffffd6 CR3: 0000000190606000 CR4:
00000000000006e0
[ 3003.840394] Call Trace:
[ 3003.840394] ? tpm_chip_start+0xb8/0xe0
[ 3003.840394] ? tpm_chip_register+0x15/0x240
[ 3003.840394] ? vtpm_proxy_work+0x15/0x30
[ 3003.840394] ? process_one_work+0x237/0x5c0
[ 3003.840394] ? worker_thread+0x1d5/0x390
[ 3003.840394] ? process_one_work+0x5c0/0x5c0
[ 3003.840394] ? kthread+0x11e/0x140
[ 3003.840394] ? kthread_park+0x90/0x90
[ 3003.840394] ? ret_from_fork+0x3a/0x50
[ 3003.840394] Modules linked in: xt_CHECKSUM ipt_MASQUERADE tun
nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter
ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat
ebtable_broute bridge stp llc ip6table_nat nf_nat_ipv6 ip6table_mangle
ip6table_raw ip6table_security iptable_nat nf_nat_ipv4 nf_nat
nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c iptable_mangle
iptable_raw iptable_security ebtable_filter ebtables ip6table_filter
ip6_tables sunrpc virtio_gpu ttm drm_kms_helper drm joydev syscopyarea
sysfillrect sysimgblt pcspkr virtio_balloon i2c_piix4 fb_sys_fops
virtio_net net_failover virtio_console failover virtio_blk crc32c_intel
virtio_pci serio_raw ata_generic virtio_ring virtio pata_acpi floppy
qemu_fw_cfg
[ 3003.840394] CR2: 0000000000000000
[ 3003.840394] ---[ end trace 29e5990b605a7ccb ]---
[ 3003.840394] RIP: 0010: (null)
[ 3003.840394] Code: Bad RIP value.
[ 3003.840394] RSP: 0018:ffffa6e6816d7e10 EFLAGS: 00010246
[ 3003.840394] RAX: 00000000ffffffe0 RBX: ffff9698077ad000 RCX:
0000000000000006
[ 3003.840394] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
ffff9698077ad000
[ 3003.840394] RBP: ffff9698077ad000 R08: fffff59bc6172d08 R09:
0000000000000000
[ 3003.840394] R10: 0000000000000000 R11: 0000000000000000 R12:
ffff9698b78f0700
[ 3003.840394] R13: ffff9698b78fbc00 R14: 0000000000000000 R15:
ffff9697f8ea10f8
[ 3003.840394] FS: 0000000000000000(0000) GS:ffff9698b78c0000(0000)
knlGS:0000000000000000
[ 3003.840394] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3003.840394] CR2: ffffffffffffffd6 CR3: 0000000190606000 CR4:
00000000000006e0
[ 3003.840394] BUG: sleeping function called from invalid context at
./include/linux/percpu-rwsem.h:34
[ 3003.840394] in_atomic(): 0, irqs_disabled(): 1, pid: 111, name:
kworker/3:1
[ 3003.840394] INFO: lockdep is turned off.
[ 3003.840394] irq event stamp: 165206
[ 3003.840394] hardirqs last enabled at (165205): [<ffffffff8b220113>]
free_unref_page+0xf3/0x1f0
[ 3003.840394] hardirqs last disabled at (165206): [<ffffffff8b0019ee>]
trace_hardirqs_off_thunk+0x1a/0x1c
[ 3003.840394] softirqs last enabled at (165182): [<ffffffff8b785e01>]
peernet2id+0x41/0x50
[ 3003.840394] softirqs last disabled at (165180): [<ffffffff8b785de2>]
peernet2id+0x22/0x50
[ 3003.840394] CPU: 3 PID: 111 Comm: kworker/3:1 Tainted: G D
4.20.0-rc2+ #6
[ 3003.840394] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.0-50-g14221cd-dirty-20181018_164544-sbct-4.pok.ibm.com
04/01/2014
[ 3003.840394] Workqueue: tpm-vtpm vtpm_proxy_work
[ 3003.840394] Call Trace:
[ 3003.840394] dump_stack+0x67/0x9b
[ 3003.840394] ___might_sleep+0x149/0x230
[ 3003.840394] exit_signals+0x20/0x210
[ 3003.840394] ? worker_thread+0x1d5/0x390
[ 3003.840394] do_exit+0xa0/0xc70
[ 3003.840394] ? process_one_work+0x5c0/0x5c0
[ 3003.840394] ? kthread+0x11e/0x140
[ 3003.840394] rewind_stack_do_exit+0x17/0x20
[ 3004.389582] tpm tpm1803: tpm_try_transmit: tpm_send: error -14
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> drivers/char/tpm/tpm-chip.c | 110 +++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm-interface.c | 87 +-----------------------
> drivers/char/tpm/tpm.h | 2 +
> 3 files changed, 115 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 157505b0f755..65f1561eba81 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -37,6 +37,116 @@ struct class *tpm_class;
> struct class *tpmrm_class;
> dev_t tpm_devt;
>
> +static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
> +{
> + int rc;
> +
> + if (flags & TPM_TRANSMIT_NESTED)
> + return 0;
> +
> + if (!chip->ops->request_locality)
> + return 0;
> +
> + rc = chip->ops->request_locality(chip, 0);
> + if (rc < 0)
> + return rc;
> +
> + chip->locality = rc;
> + return 0;
> +}
> +
> +static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
> +{
> + int rc;
> +
> + if (flags & TPM_TRANSMIT_NESTED)
> + return;
> +
> + if (!chip->ops->relinquish_locality)
> + return;
> +
> + rc = chip->ops->relinquish_locality(chip, chip->locality);
> + if (rc)
> + dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
> +
> + chip->locality = -1;
> +}
> +
> +static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
> +{
> + if (flags & TPM_TRANSMIT_NESTED)
> + return 0;
> +
> + if (!chip->ops->cmd_ready)
> + return 0;
> +
> + return chip->ops->cmd_ready(chip);
> +}
> +
> +static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
> +{
> + if (flags & TPM_TRANSMIT_NESTED)
> + return 0;
> +
> + if (!chip->ops->go_idle)
> + return 0;
> +
> + return chip->ops->go_idle(chip);
> +}
> +
> +/**
> + * tpm_chip_start() - power on the TPM
> + * @chip: a TPM chip to use
> + * @flags: TPM transmit flags
> + *
> + * Return:
> + * * The response length - OK
> + * * -errno - A system error
> + */
> +int tpm_chip_start(struct tpm_chip *chip, unsigned int flags)
> +{
> + int ret;
> +
> + if (chip->ops->clk_enable)
> + chip->ops->clk_enable(chip, true);
> +
> + if (chip->locality == -1) {
> + ret = tpm_request_locality(chip, flags);
> + if (ret) {
> + chip->ops->clk_enable(chip, false);
> + return ret;
> + }
> + }
> +
> + ret = tpm_cmd_ready(chip, flags);
> + if (ret) {
> + tpm_relinquish_locality(chip, flags);
> + if (chip->ops->clk_enable)
> + chip->ops->clk_enable(chip, false);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * tpm_chip_stop() - power off the TPM
> + * @chip: a TPM chip to use
> + * @flags: TPM transmit flags
> + *
> + * Return:
> + * * The response length - OK
> + * * -errno - A system error
> + */
> +void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags)
> +{
> + tpm_go_idle(chip, flags);
> + tpm_relinquish_locality(chip, flags);
> + if (chip->ops->clk_enable)
> + chip->ops->clk_enable(chip, false);
> +}
> +
> +
> /**
> * tpm_try_get_ops() - Get a ref to the tpm_chip
> * @chip: Chip to ref
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 5865b9671d20..888c9923fca1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -62,64 +62,6 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> }
> EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>
> -static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
> -{
> - int rc;
> -
> - if (flags & TPM_TRANSMIT_NESTED)
> - return 0;
> -
> - if (!chip->ops->request_locality)
> - return 0;
> -
> - rc = chip->ops->request_locality(chip, 0);
> - if (rc < 0)
> - return rc;
> -
> - chip->locality = rc;
> -
> - return 0;
> -}
> -
> -static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
> -{
> - int rc;
> -
> - if (flags & TPM_TRANSMIT_NESTED)
> - return;
> -
> - if (!chip->ops->relinquish_locality)
> - return;
> -
> - rc = chip->ops->relinquish_locality(chip, chip->locality);
> - if (rc)
> - dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
> -
> - chip->locality = -1;
> -}
> -
> -static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
> -{
> - if (flags & TPM_TRANSMIT_NESTED)
> - return 0;
> -
> - if (!chip->ops->cmd_ready)
> - return 0;
> -
> - return chip->ops->cmd_ready(chip);
> -}
> -
> -static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
> -{
> - if (flags & TPM_TRANSMIT_NESTED)
> - return 0;
> -
> - if (!chip->ops->go_idle)
> - return 0;
> -
> - return chip->ops->go_idle(chip);
> -}
> -
> static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
> unsigned int flags)
> {
> @@ -212,7 +154,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
> /* space for header and handles */
> u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
> unsigned int delay_msec = TPM2_DURATION_SHORT;
> - bool has_locality = false;
> u32 rc = 0;
> ssize_t ret;
> const size_t save_size = min(sizeof(save), bufsiz);
> @@ -227,34 +168,12 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
> memcpy(save, buf, save_size);
>
> for (;;) {
> - if (chip->ops->clk_enable != NULL)
> - chip->ops->clk_enable(chip, true);
> -
> - if (chip->locality == -1) {
> - ret = tpm_request_locality(chip, flags);
> - if (ret)
> - goto out_locality;
> - has_locality = true;
> - }
> -
> - ret = tpm_cmd_ready(chip, flags);
> + ret = tpm_chip_start(chip, flags);
> if (ret)
> - goto out_locality;
> -
> + return ret;
> ret = tpm_try_transmit(chip, buf, bufsiz, flags);
> + tpm_chip_stop(chip, flags);
>
> - /* This may fail but do not override ret. */
> - tpm_go_idle(chip, flags);
> -
> -out_locality:
> - if (has_locality)
> - tpm_relinquish_locality(chip, flags);
> -
> - if (chip->ops->clk_enable != NULL)
> - chip->ops->clk_enable(chip, false);
> -
> - if (ret < 0)
> - break;
> rc = be32_to_cpu(header->return_code);
> if (rc != TPM2_RC_RETRY && rc != TPM2_RC_TESTING)
> break;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index c7c06de651a0..c42a75710b70 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -523,6 +523,8 @@ static inline void tpm_msleep(unsigned int delay_msec)
> delay_msec * 1000);
> };
>
> +int tpm_chip_start(struct tpm_chip *chip, unsigned int flags);
> +void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags);
> struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
> __must_check int tpm_try_get_ops(struct tpm_chip *chip);
> void tpm_put_ops(struct tpm_chip *chip);
next prev parent reply other threads:[~2018-11-13 22:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-13 18:35 [PATCH v7 00/17] Removed nested TPM operations Jarkko Sakkinen
2018-11-13 18:35 ` Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 01/17] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 02/17] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 03/17] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 04/17] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 05/17] tpm: declare struct tpm_header Jarkko Sakkinen
2018-11-13 18:36 ` Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 06/17] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 07/17] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 08/17] tpm: call tpm2_flush_space() on error in tpm_try_transmit() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 09/17] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 10/17] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 11/17] tpm: move TPM space code out of tpm_transmit() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 12/17] tpm: remove @space from tpm_transmit() Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 13/17] tpm: use tpm_try_get_ops() in tpm-sysfs.c Jarkko Sakkinen
2018-11-13 18:36 ` [PATCH v7 14/17] tpm: remove TPM_TRANSMIT_UNLOCKED flag Jarkko Sakkinen
2018-11-13 20:39 ` Stefan Berger
2018-11-13 18:36 ` [PATCH v7 15/17] tpm: introduce tpm_chip_start() and tpm_chip_stop() Jarkko Sakkinen
2018-11-13 21:17 ` Stefan Berger
2018-11-13 22:34 ` Stefan Berger [this message]
2018-11-14 1:56 ` Stefan Berger
2018-11-16 12:32 ` Jarkko Sakkinen
2018-11-15 20:26 ` Jarkko Sakkinen
2018-11-13 23:02 ` Stefan Berger
2018-11-14 1:47 ` Stefan Berger
2018-11-13 18:36 ` [PATCH v7 16/17] tpm: take TPM chip power gating out of tpm_transmit() Jarkko Sakkinen
2018-11-13 21:26 ` Stefan Berger
2018-11-13 18:36 ` [PATCH v7 17/17] tpm: remove @flags from tpm_transmit() Jarkko Sakkinen
2018-11-13 21:28 ` Stefan Berger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f13401b7-e683-6b61-b0cf-7c4be87903cb@linux.ibm.com \
--to=stefanb@linux.ibm.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jgg@ziepe.ca \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=nayna@linux.ibm.com \
--cc=peterhuewe@gmx.de \
--cc=stefanb@linux.vnet.ibm.com \
--cc=tadeusz.struk@intel.com \
--cc=tomas.winkler@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.