* [PATCH v4 0/2] tpm: Unify send() callbacks
@ 2019-02-08 18:08 Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 18:08 UTC (permalink / raw)
To: linux-integrity
Cc: linux-kernel, linux-security-module, Peter Huewe,
Jason Gunthorpe, Stefan Berger, Alexander Steffen,
Jarkko Sakkinen
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.co m>
A portion of send() callbacks have returned length, in many cases just
returning back what was given as an argument, and tpm_crb has returned 0 on
success. This patch set fixes and unifies the behaviour.
v4:
* Return zero already in tpm_tis_send_main() so that it is applied to all
call sites of tpm_tis_send().
* Fixup tpm_ibmvtpm_send() documentation.
v3:
tpm_tis_core fix was left out of the staging area :-(
v2:
The drivers tpm_nsc and tpm_infineon were forgotten. For this version I
checked both with find and command and from Kconfig that everything that is
supposed to be a driver directly interfacing with the TPM core, is included
(e.g. discluding tpm_tis_spi).
Jarkko Sakkinen (2):
tpm: Unify the send callback behaviour
tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete
drivers/char/tpm/st33zp24/st33zp24.c | 2 +-
drivers/char/tpm/tpm_atmel.c | 2 +-
drivers/char/tpm/tpm_i2c_atmel.c | 10 +++++++++-
drivers/char/tpm/tpm_i2c_infineon.c | 2 +-
drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +-
drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++----
drivers/char/tpm/tpm_infineon.c | 2 +-
drivers/char/tpm/tpm_nsc.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 2 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
drivers/char/tpm/xen-tpmfront.c | 2 +-
11 files changed, 22 insertions(+), 15 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-08 18:08 [PATCH v4 0/2] tpm: Unify send() callbacks Jarkko Sakkinen
@ 2019-02-08 18:08 ` Jarkko Sakkinen
2019-02-08 18:12 ` Stefan Berger
2019-02-09 18:20 ` Jerry Snitselaar
2019-02-08 18:08 ` [PATCH v4 2/2] tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete Jarkko Sakkinen
2019-02-09 20:08 ` [PATCH v4 0/2] tpm: Unify send() callbacks Jerry Snitselaar
2 siblings, 2 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 18:08 UTC (permalink / raw)
To: linux-integrity
Cc: linux-kernel, linux-security-module, Peter Huewe,
Jason Gunthorpe, Stefan Berger, Alexander Steffen,
Jarkko Sakkinen, stable
The send() callback should never return length as it does not in every
driver except tpm_crb in the success case. The reason is that the main
transmit functionality only cares about whether the transmit was
successful or not and ignores the count completely.
Cc: stable@vger.kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/st33zp24/st33zp24.c | 2 +-
drivers/char/tpm/tpm_atmel.c | 2 +-
drivers/char/tpm/tpm_i2c_atmel.c | 6 +++++-
drivers/char/tpm/tpm_i2c_infineon.c | 2 +-
drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +-
drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++----
drivers/char/tpm/tpm_infineon.c | 2 +-
drivers/char/tpm/tpm_nsc.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 2 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
drivers/char/tpm/xen-tpmfront.c | 2 +-
11 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 64dc560859f2..13dc614b7ebc 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -436,7 +436,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
goto out_err;
}
- return len;
+ return 0;
out_err:
st33zp24_cancel(chip);
release_locality(chip);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 66a14526aaf4..a290b30a0c35 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -105,7 +105,7 @@ static int tpm_atml_send(struct tpm_chip *chip, u8 *buf, size_t count)
iowrite8(buf[i], priv->iobase);
}
- return count;
+ return 0;
}
static void tpm_atml_cancel(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 4720b2442ffe..aa11c8a1df5e 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -65,7 +65,11 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
dev_dbg(&chip->dev,
"%s(buf=%*ph len=%0zx) -> sts=%d\n", __func__,
(int)min_t(size_t, 64, len), buf, len, status);
- return status;
+
+ if (status < 0)
+ return status;
+
+ return 0;
}
static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 3b490d9d90e7..3b4e9672ff6c 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -588,7 +588,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
/* go and do it */
iic_tpm_write(TPM_STS(tpm_dev.locality), &sts, 1);
- return len;
+ return 0;
out_err:
tpm_tis_i2c_ready(chip);
/* The TPM needs some time to clean up here,
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 5700cc2ddee1..315a3b4548f7 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -465,7 +465,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
}
dev_dbg(dev, "%s() -> %zd\n", __func__, len);
- return len;
+ return 0;
}
static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 07b5a487d0c8..757ca45b39b8 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -139,14 +139,14 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
}
/**
- * tpm_ibmvtpm_send - Send tpm request
- *
+ * tpm_ibmvtpm_send() - Send a TPM command
* @chip: tpm chip struct
* @buf: buffer contains data to send
* @count: size of buffer
*
* Return:
- * Number of bytes sent or < 0 on error.
+ * 0 on success,
+ * -errno on error
*/
static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
{
@@ -192,7 +192,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
rc = 0;
ibmvtpm->tpm_processing_cmd = false;
} else
- rc = count;
+ rc = 0;
spin_unlock(&ibmvtpm->rtce_lock);
return rc;
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index d8f10047fbba..97f6d4fe0aee 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -354,7 +354,7 @@ static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count)
for (i = 0; i < count; i++) {
wait_and_send(chip, buf[i]);
}
- return count;
+ return 0;
}
static void tpm_inf_cancel(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 5d6cce74cd3f..9bee3c5eb4bf 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -226,7 +226,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
}
outb(NSC_COMMAND_EOC, priv->base + NSC_COMMAND);
- return count;
+ return 0;
}
static void tpm_nsc_cancel(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 60e2038652b8..b9f64684c3fb 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -481,7 +481,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
goto out_err;
}
}
- return len;
+ return 0;
out_err:
tpm_tis_ready(chip);
return rc;
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 26a2be555288..d74f3de74ae6 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -335,7 +335,6 @@ static int vtpm_proxy_is_driver_command(struct tpm_chip *chip,
static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
{
struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
- int rc = 0;
if (count > sizeof(proxy_dev->buffer)) {
dev_err(&chip->dev,
@@ -366,7 +365,7 @@ static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
wake_up_interruptible(&proxy_dev->wq);
- return rc;
+ return 0;
}
static void vtpm_proxy_tpm_op_cancel(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 1259e935fd58..4e2d00cb0d81 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -173,7 +173,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
return -ETIME;
}
- return count;
+ return 0;
}
static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 2/2] tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete
2019-02-08 18:08 [PATCH v4 0/2] tpm: Unify send() callbacks Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
@ 2019-02-08 18:08 ` Jarkko Sakkinen
2019-02-09 20:08 ` [PATCH v4 0/2] tpm: Unify send() callbacks Jerry Snitselaar
2 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 18:08 UTC (permalink / raw)
To: linux-integrity
Cc: linux-kernel, linux-security-module, Peter Huewe,
Jason Gunthorpe, Stefan Berger, Alexander Steffen,
Jarkko Sakkinen, stable
Return -E2BIG when the transfer is incomplete. The upper layer does
not retry, so not doing that is incorrect behaviour.
Cc: stable@vger.kernel.org
Fixes: a2871c62e186 ("tpm: Add support for Atmel I2C TPMs")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
drivers/char/tpm/tpm_i2c_atmel.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index aa11c8a1df5e..8a7e80923091 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -69,6 +69,10 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
if (status < 0)
return status;
+ /* The upper layer does not support incomplete sends. */
+ if (status != len)
+ return -E2BIG;
+
return 0;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
@ 2019-02-08 18:12 ` Stefan Berger
2019-02-08 19:00 ` Jarkko Sakkinen
2019-02-09 18:20 ` Jerry Snitselaar
1 sibling, 1 reply; 20+ messages in thread
From: Stefan Berger @ 2019-02-08 18:12 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: linux-kernel, linux-security-module, Peter Huewe,
Jason Gunthorpe, Stefan Berger, Alexander Steffen, stable
On 2/8/19 1:08 PM, Jarkko Sakkinen wrote:
> The send() callback should never return length as it does not in every
> driver except tpm_crb in the success case. The reason is that the main
> transmit functionality only cares about whether the transmit was
> successful or not and ignores the count completely.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Let me know when you put it into your tree, I'll give it a spin while I
am at it. :-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-08 18:12 ` Stefan Berger
@ 2019-02-08 19:00 ` Jarkko Sakkinen
2019-02-08 19:17 ` [PATCH v4 1/2] tpm: Unify the send callback behaviourä Jarkko Sakkinen
2019-02-11 15:05 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Alexander Steffen
0 siblings, 2 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 19:00 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 01:12:34PM -0500, Stefan Berger wrote:
> On 2/8/19 1:08 PM, Jarkko Sakkinen wrote:
> > The send() callback should never return length as it does not in every
> > driver except tpm_crb in the success case. The reason is that the main
> > transmit functionality only cares about whether the transmit was
> > successful or not and ignores the count completely.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
> Let me know when you put it into your tree, I'll give it a spin while I am
> at it. :-)
Thank you Stefan! I also add your suggested-by to the first commit
because you pointed out the problem.
It all looks now legit, but just in case I'll add a check for the return
value to tpm_try_transmit() and a warning if it is not zero in the
success case (and after that zeroing of rc).
That check can be removed when I do v5.3 pull request. That should
enough window to catch any potential issues and check will ensure that
kernel won't fail even there was something forgotten.
Alexander, I'll push this version now to the master and next with the
additional check described in this commit, but will add your tags
after you have time to test.
Thanks alot!
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 19:00 ` Jarkko Sakkinen
@ 2019-02-08 19:17 ` Jarkko Sakkinen
2019-02-08 19:27 ` Stefan Berger
2019-02-11 15:05 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Alexander Steffen
1 sibling, 1 reply; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 19:17 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 09:00:57PM +0200, Jarkko Sakkinen wrote:
> It all looks now legit, but just in case I'll add a check for the return
> value to tpm_try_transmit() and a warning if it is not zero in the
> success case (and after that zeroing of rc).
Now the commits are applied both master and next, and these are
the checks for send():
rc = chip->ops->send(chip, buf, count);
if (rc < 0) {
if (rc != -EPIPE)
dev_err(&chip->dev,
"%s: send(): error %d\n", __func__, rc);
return rc;
}
/* A sanity check. send() should just return zero on success e.g.
* not the command length.
*/
if (rc > 0) {
dev_warn(&chip->dev,
"%s: send(): invalid value %d\n", __func__, rc);
rc = 0;
}
Should be fairly safe play now.
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 19:17 ` [PATCH v4 1/2] tpm: Unify the send callback behaviourä Jarkko Sakkinen
@ 2019-02-08 19:27 ` Stefan Berger
2019-02-08 20:23 ` Jarkko Sakkinen
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Berger @ 2019-02-08 19:27 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On 2/8/19 2:17 PM, Jarkko Sakkinen wrote:
>
> */
> if (rc > 0) {
> dev_warn(&chip->dev,
> "%s: send(): invalid value %d\n", __func__, rc);
> rc = 0;
> }
>
> Should be fairly safe play now.
Unfortuantely it isn't. You seemed to have lost the
EXPORT_SYMBOL_GPL(tpm_chip_start/stop) and the tpm_chip_start/stop
around the tpm2_shutdown()...
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 19:27 ` Stefan Berger
@ 2019-02-08 20:23 ` Jarkko Sakkinen
2019-02-08 20:32 ` Stefan Berger
2019-02-08 20:38 ` Jarkko Sakkinen
0 siblings, 2 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 20:23 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 02:27:31PM -0500, Stefan Berger wrote:
> On 2/8/19 2:17 PM, Jarkko Sakkinen wrote:
> >
> > */
> > if (rc > 0) {
> > dev_warn(&chip->dev,
> > "%s: send(): invalid value %d\n", __func__, rc);
> > rc = 0;
> > }
> >
> > Should be fairly safe play now.
>
> Unfortuantely it isn't. You seemed to have lost the
> EXPORT_SYMBOL_GPL(tpm_chip_start/stop) and the tpm_chip_start/stop around
> the tpm2_shutdown()...
OK, now those fixes are back. In tpm_pm_shutdown() case you need also
take the lock.
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 20:23 ` Jarkko Sakkinen
@ 2019-02-08 20:32 ` Stefan Berger
2019-02-08 20:46 ` Jarkko Sakkinen
2019-02-08 20:38 ` Jarkko Sakkinen
1 sibling, 1 reply; 20+ messages in thread
From: Stefan Berger @ 2019-02-08 20:32 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On 2/8/19 3:23 PM, Jarkko Sakkinen wrote:
> On Fri, Feb 08, 2019 at 02:27:31PM -0500, Stefan Berger wrote:
>> On 2/8/19 2:17 PM, Jarkko Sakkinen wrote:
>>> */
>>> if (rc > 0) {
>>> dev_warn(&chip->dev,
>>> "%s: send(): invalid value %d\n", __func__, rc);
>>> rc = 0;
>>> }
>>>
>>> Should be fairly safe play now.
>> Unfortuantely it isn't. You seemed to have lost the
>> EXPORT_SYMBOL_GPL(tpm_chip_start/stop) and the tpm_chip_start/stop around
>> the tpm2_shutdown()...
> OK, now those fixes are back. In tpm_pm_shutdown() case you need also
> take the lock.
tpm_del_char_device also needs the start/stop!
Stefan
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 20:23 ` Jarkko Sakkinen
2019-02-08 20:32 ` Stefan Berger
@ 2019-02-08 20:38 ` Jarkko Sakkinen
1 sibling, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 20:38 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 10:23:53PM +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 08, 2019 at 02:27:31PM -0500, Stefan Berger wrote:
> > On 2/8/19 2:17 PM, Jarkko Sakkinen wrote:
> > >
> > > */
> > > if (rc > 0) {
> > > dev_warn(&chip->dev,
> > > "%s: send(): invalid value %d\n", __func__, rc);
> > > rc = 0;
> > > }
> > >
> > > Should be fairly safe play now.
> >
> > Unfortuantely it isn't. You seemed to have lost the
> > EXPORT_SYMBOL_GPL(tpm_chip_start/stop) and the tpm_chip_start/stop around
> > the tpm2_shutdown()...
>
> OK, now those fixes are back. In tpm_pm_shutdown() case you need also
> take the lock.
I back tracked now what went wrong. When I first did these fixes I did
git reset --hard next on master after doing them and not the other way
around I supposed to. Apologies for the inconvenience...
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 20:32 ` Stefan Berger
@ 2019-02-08 20:46 ` Jarkko Sakkinen
2019-02-08 21:18 ` Stefan Berger
0 siblings, 1 reply; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 20:46 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 03:32:32PM -0500, Stefan Berger wrote:
> On 2/8/19 3:23 PM, Jarkko Sakkinen wrote:
> > On Fri, Feb 08, 2019 at 02:27:31PM -0500, Stefan Berger wrote:
> > > On 2/8/19 2:17 PM, Jarkko Sakkinen wrote:
> > > > */
> > > > if (rc > 0) {
> > > > dev_warn(&chip->dev,
> > > > "%s: send(): invalid value %d\n", __func__, rc);
> > > > rc = 0;
> > > > }
> > > >
> > > > Should be fairly safe play now.
> > > Unfortuantely it isn't. You seemed to have lost the
> > > EXPORT_SYMBOL_GPL(tpm_chip_start/stop) and the tpm_chip_start/stop around
> > > the tpm2_shutdown()...
> > OK, now those fixes are back. In tpm_pm_shutdown() case you need also
> > take the lock.
>
> tpm_del_char_device also needs the start/stop!
Done and updated the commit message to have all the call sites:
* tpm_chip_register()
* tpm_class_shutdown()
* tpm_del_char_device()
* tpm_pm_suspend()
* tpm_try_get_ops() and tpm_put_ops()
* tpm2_del_space()
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 20:46 ` Jarkko Sakkinen
@ 2019-02-08 21:18 ` Stefan Berger
2019-02-08 21:51 ` Jarkko Sakkinen
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Berger @ 2019-02-08 21:18 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On 2/8/19 3:46 PM, Jarkko Sakkinen wrote:
> On Fri, Feb 08, 2019 at 03:32:32PM -0500, Stefan Berger wrote:
>>
>> tpm_del_char_device also needs the start/stop!
> Done and updated the commit message to have all the call sites:
>
> * tpm_chip_register()
> * tpm_class_shutdown()
> * tpm_del_char_device()
> * tpm_pm_suspend()
> * tpm_try_get_ops() and tpm_put_ops()
> * tpm2_del_space()
I tested this now with tpm_vtpm_proxy, TPM 1.2, (TIS|CRB) + TPM 2.0.
Looks good now - rock solid so to say. And while we are at it ... :-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviourä
2019-02-08 21:18 ` Stefan Berger
@ 2019-02-08 21:51 ` Jarkko Sakkinen
0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-08 21:51 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri, Feb 08, 2019 at 04:18:40PM -0500, Stefan Berger wrote:
> On 2/8/19 3:46 PM, Jarkko Sakkinen wrote:
> > On Fri, Feb 08, 2019 at 03:32:32PM -0500, Stefan Berger wrote:
> > >
> > > tpm_del_char_device also needs the start/stop!
> > Done and updated the commit message to have all the call sites:
> >
> > * tpm_chip_register()
> > * tpm_class_shutdown()
> > * tpm_del_char_device()
> > * tpm_pm_suspend()
> > * tpm_try_get_ops() and tpm_put_ops()
> > * tpm2_del_space()
>
>
> I tested this now with tpm_vtpm_proxy, TPM 1.2, (TIS|CRB) + TPM 2.0. Looks
> good now - rock solid so to say. And while we are at it ... :-)
Thank you for your support. I don't think these quite distruptive
changes would have made in without any growing pains. The way
tpm_transmit() used to be had become quite a mess with all the nested
calls and weird locking flags. Now we have a good baseline to forward
:-)
I rebased the branches to the latest security/next-general. Probably
do the PR after the middle week (ETA Thu).
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
2019-02-08 18:12 ` Stefan Berger
@ 2019-02-09 18:20 ` Jerry Snitselaar
2019-02-09 20:01 ` Jerry Snitselaar
2019-02-11 13:58 ` Jarkko Sakkinen
1 sibling, 2 replies; 20+ messages in thread
From: Jerry Snitselaar @ 2019-02-09 18:20 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Fri Feb 08 19, Jarkko Sakkinen wrote:
>The send() callback should never return length as it does not in every
>driver except tpm_crb in the success case. The reason is that the main
>transmit functionality only cares about whether the transmit was
>successful or not and ignores the count completely.
>
>Cc: stable@vger.kernel.org
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>---
> drivers/char/tpm/st33zp24/st33zp24.c | 2 +-
> drivers/char/tpm/tpm_atmel.c | 2 +-
> drivers/char/tpm/tpm_i2c_atmel.c | 6 +++++-
> drivers/char/tpm/tpm_i2c_infineon.c | 2 +-
> drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +-
> drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++----
> drivers/char/tpm/tpm_infineon.c | 2 +-
> drivers/char/tpm/tpm_nsc.c | 2 +-
> drivers/char/tpm/tpm_tis_core.c | 2 +-
> drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
> drivers/char/tpm/xen-tpmfront.c | 2 +-
> 11 files changed, 18 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
>index 64dc560859f2..13dc614b7ebc 100644
>--- a/drivers/char/tpm/st33zp24/st33zp24.c
>+++ b/drivers/char/tpm/st33zp24/st33zp24.c
>@@ -436,7 +436,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
> goto out_err;
> }
>
>- return len;
>+ return 0;
> out_err:
> st33zp24_cancel(chip);
> release_locality(chip);
>diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
>index 66a14526aaf4..a290b30a0c35 100644
>--- a/drivers/char/tpm/tpm_atmel.c
>+++ b/drivers/char/tpm/tpm_atmel.c
>@@ -105,7 +105,7 @@ static int tpm_atml_send(struct tpm_chip *chip, u8 *buf, size_t count)
> iowrite8(buf[i], priv->iobase);
> }
>
>- return count;
>+ return 0;
> }
>
> static void tpm_atml_cancel(struct tpm_chip *chip)
>diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
>index 4720b2442ffe..aa11c8a1df5e 100644
>--- a/drivers/char/tpm/tpm_i2c_atmel.c
>+++ b/drivers/char/tpm/tpm_i2c_atmel.c
>@@ -65,7 +65,11 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
> dev_dbg(&chip->dev,
> "%s(buf=%*ph len=%0zx) -> sts=%d\n", __func__,
> (int)min_t(size_t, 64, len), buf, len, status);
>- return status;
>+
>+ if (status < 0)
>+ return status;
>+
>+ return 0;
> }
>
> static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
>index 3b490d9d90e7..3b4e9672ff6c 100644
>--- a/drivers/char/tpm/tpm_i2c_infineon.c
>+++ b/drivers/char/tpm/tpm_i2c_infineon.c
>@@ -588,7 +588,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
> /* go and do it */
> iic_tpm_write(TPM_STS(tpm_dev.locality), &sts, 1);
>
>- return len;
>+ return 0;
> out_err:
> tpm_tis_i2c_ready(chip);
> /* The TPM needs some time to clean up here,
>diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
>index 5700cc2ddee1..315a3b4548f7 100644
>--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
>+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
>@@ -465,7 +465,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
> }
>
> dev_dbg(dev, "%s() -> %zd\n", __func__, len);
>- return len;
>+ return 0;
> }
>
> static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
>diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
>index 07b5a487d0c8..757ca45b39b8 100644
>--- a/drivers/char/tpm/tpm_ibmvtpm.c
>+++ b/drivers/char/tpm/tpm_ibmvtpm.c
>@@ -139,14 +139,14 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> }
>
> /**
>- * tpm_ibmvtpm_send - Send tpm request
>- *
>+ * tpm_ibmvtpm_send() - Send a TPM command
> * @chip: tpm chip struct
> * @buf: buffer contains data to send
> * @count: size of buffer
> *
> * Return:
>- * Number of bytes sent or < 0 on error.
>+ * 0 on success,
>+ * -errno on error
> */
> static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> {
>@@ -192,7 +192,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> rc = 0;
> ibmvtpm->tpm_processing_cmd = false;
> } else
>- rc = count;
>+ rc = 0;
>
> spin_unlock(&ibmvtpm->rtce_lock);
> return rc;
>diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
>index d8f10047fbba..97f6d4fe0aee 100644
>--- a/drivers/char/tpm/tpm_infineon.c
>+++ b/drivers/char/tpm/tpm_infineon.c
>@@ -354,7 +354,7 @@ static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count)
> for (i = 0; i < count; i++) {
> wait_and_send(chip, buf[i]);
> }
>- return count;
>+ return 0;
> }
>
> static void tpm_inf_cancel(struct tpm_chip *chip)
>diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
>index 5d6cce74cd3f..9bee3c5eb4bf 100644
>--- a/drivers/char/tpm/tpm_nsc.c
>+++ b/drivers/char/tpm/tpm_nsc.c
>@@ -226,7 +226,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
> }
> outb(NSC_COMMAND_EOC, priv->base + NSC_COMMAND);
>
>- return count;
>+ return 0;
> }
>
> static void tpm_nsc_cancel(struct tpm_chip *chip)
>diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>index 60e2038652b8..b9f64684c3fb 100644
>--- a/drivers/char/tpm/tpm_tis_core.c
>+++ b/drivers/char/tpm/tpm_tis_core.c
>@@ -481,7 +481,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
> goto out_err;
> }
> }
>- return len;
>+ return 0;
> out_err:
> tpm_tis_ready(chip);
> return rc;
>diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>index 26a2be555288..d74f3de74ae6 100644
>--- a/drivers/char/tpm/tpm_vtpm_proxy.c
>+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>@@ -335,7 +335,6 @@ static int vtpm_proxy_is_driver_command(struct tpm_chip *chip,
> static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
> {
> struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
>- int rc = 0;
>
> if (count > sizeof(proxy_dev->buffer)) {
> dev_err(&chip->dev,
>@@ -366,7 +365,7 @@ static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
>
> wake_up_interruptible(&proxy_dev->wq);
>
>- return rc;
>+ return 0;
> }
>
> static void vtpm_proxy_tpm_op_cancel(struct tpm_chip *chip)
>diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
>index 1259e935fd58..4e2d00cb0d81 100644
>--- a/drivers/char/tpm/xen-tpmfront.c
>+++ b/drivers/char/tpm/xen-tpmfront.c
>@@ -173,7 +173,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> return -ETIME;
> }
>
>- return count;
>+ return 0;
> }
>
> static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>--
>2.19.1
>
Does st33zp24_i2c_send need an update as well? It does 'return write8_reg()'.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-09 18:20 ` Jerry Snitselaar
@ 2019-02-09 20:01 ` Jerry Snitselaar
2019-02-11 13:58 ` Jarkko Sakkinen
1 sibling, 0 replies; 20+ messages in thread
From: Jerry Snitselaar @ 2019-02-09 20:01 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Linux List Kernel Mailing,
linux-security-module, Peter Huewe, Jason Gunthorpe,
Stefan Berger, Alexander Steffen, stable
On Sat, Feb 9, 2019 at 11:20 AM Jerry Snitselaar <jsnitsel@redhat.com> wrote:
>
> On Fri Feb 08 19, Jarkko Sakkinen wrote:
> >The send() callback should never return length as it does not in every
> >driver except tpm_crb in the success case. The reason is that the main
> >transmit functionality only cares about whether the transmit was
> >successful or not and ignores the count completely.
> >
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >---
> > drivers/char/tpm/st33zp24/st33zp24.c | 2 +-
> > drivers/char/tpm/tpm_atmel.c | 2 +-
> > drivers/char/tpm/tpm_i2c_atmel.c | 6 +++++-
> > drivers/char/tpm/tpm_i2c_infineon.c | 2 +-
> > drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +-
> > drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++----
> > drivers/char/tpm/tpm_infineon.c | 2 +-
> > drivers/char/tpm/tpm_nsc.c | 2 +-
> > drivers/char/tpm/tpm_tis_core.c | 2 +-
> > drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
> > drivers/char/tpm/xen-tpmfront.c | 2 +-
> > 11 files changed, 18 insertions(+), 15 deletions(-)
> >
> >diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> >index 64dc560859f2..13dc614b7ebc 100644
> >--- a/drivers/char/tpm/st33zp24/st33zp24.c
> >+++ b/drivers/char/tpm/st33zp24/st33zp24.c
> >@@ -436,7 +436,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
> > goto out_err;
> > }
> >
> >- return len;
> >+ return 0;
> > out_err:
> > st33zp24_cancel(chip);
> > release_locality(chip);
> >diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> >index 66a14526aaf4..a290b30a0c35 100644
> >--- a/drivers/char/tpm/tpm_atmel.c
> >+++ b/drivers/char/tpm/tpm_atmel.c
> >@@ -105,7 +105,7 @@ static int tpm_atml_send(struct tpm_chip *chip, u8 *buf, size_t count)
> > iowrite8(buf[i], priv->iobase);
> > }
> >
> >- return count;
> >+ return 0;
> > }
> >
> > static void tpm_atml_cancel(struct tpm_chip *chip)
> >diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
> >index 4720b2442ffe..aa11c8a1df5e 100644
> >--- a/drivers/char/tpm/tpm_i2c_atmel.c
> >+++ b/drivers/char/tpm/tpm_i2c_atmel.c
> >@@ -65,7 +65,11 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
> > dev_dbg(&chip->dev,
> > "%s(buf=%*ph len=%0zx) -> sts=%d\n", __func__,
> > (int)min_t(size_t, 64, len), buf, len, status);
> >- return status;
> >+
> >+ if (status < 0)
> >+ return status;
> >+
> >+ return 0;
> > }
> >
> > static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> >diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> >index 3b490d9d90e7..3b4e9672ff6c 100644
> >--- a/drivers/char/tpm/tpm_i2c_infineon.c
> >+++ b/drivers/char/tpm/tpm_i2c_infineon.c
> >@@ -588,7 +588,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
> > /* go and do it */
> > iic_tpm_write(TPM_STS(tpm_dev.locality), &sts, 1);
> >
> >- return len;
> >+ return 0;
> > out_err:
> > tpm_tis_i2c_ready(chip);
> > /* The TPM needs some time to clean up here,
> >diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
> >index 5700cc2ddee1..315a3b4548f7 100644
> >--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> >+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> >@@ -465,7 +465,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
> > }
> >
> > dev_dbg(dev, "%s() -> %zd\n", __func__, len);
> >- return len;
> >+ return 0;
> > }
> >
> > static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
> >diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> >index 07b5a487d0c8..757ca45b39b8 100644
> >--- a/drivers/char/tpm/tpm_ibmvtpm.c
> >+++ b/drivers/char/tpm/tpm_ibmvtpm.c
> >@@ -139,14 +139,14 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> > }
> >
> > /**
> >- * tpm_ibmvtpm_send - Send tpm request
> >- *
> >+ * tpm_ibmvtpm_send() - Send a TPM command
> > * @chip: tpm chip struct
> > * @buf: buffer contains data to send
> > * @count: size of buffer
> > *
> > * Return:
> >- * Number of bytes sent or < 0 on error.
> >+ * 0 on success,
> >+ * -errno on error
> > */
> > static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> > {
> >@@ -192,7 +192,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> > rc = 0;
> > ibmvtpm->tpm_processing_cmd = false;
> > } else
> >- rc = count;
> >+ rc = 0;
> >
> > spin_unlock(&ibmvtpm->rtce_lock);
> > return rc;
> >diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> >index d8f10047fbba..97f6d4fe0aee 100644
> >--- a/drivers/char/tpm/tpm_infineon.c
> >+++ b/drivers/char/tpm/tpm_infineon.c
> >@@ -354,7 +354,7 @@ static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count)
> > for (i = 0; i < count; i++) {
> > wait_and_send(chip, buf[i]);
> > }
> >- return count;
> >+ return 0;
> > }
> >
> > static void tpm_inf_cancel(struct tpm_chip *chip)
> >diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
> >index 5d6cce74cd3f..9bee3c5eb4bf 100644
> >--- a/drivers/char/tpm/tpm_nsc.c
> >+++ b/drivers/char/tpm/tpm_nsc.c
> >@@ -226,7 +226,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
> > }
> > outb(NSC_COMMAND_EOC, priv->base + NSC_COMMAND);
> >
> >- return count;
> >+ return 0;
> > }
> >
> > static void tpm_nsc_cancel(struct tpm_chip *chip)
> >diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> >index 60e2038652b8..b9f64684c3fb 100644
> >--- a/drivers/char/tpm/tpm_tis_core.c
> >+++ b/drivers/char/tpm/tpm_tis_core.c
> >@@ -481,7 +481,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
> > goto out_err;
> > }
> > }
> >- return len;
> >+ return 0;
> > out_err:
> > tpm_tis_ready(chip);
> > return rc;
> >diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> >index 26a2be555288..d74f3de74ae6 100644
> >--- a/drivers/char/tpm/tpm_vtpm_proxy.c
> >+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> >@@ -335,7 +335,6 @@ static int vtpm_proxy_is_driver_command(struct tpm_chip *chip,
> > static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
> > {
> > struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> >- int rc = 0;
> >
> > if (count > sizeof(proxy_dev->buffer)) {
> > dev_err(&chip->dev,
> >@@ -366,7 +365,7 @@ static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
> >
> > wake_up_interruptible(&proxy_dev->wq);
> >
> >- return rc;
> >+ return 0;
> > }
> >
> > static void vtpm_proxy_tpm_op_cancel(struct tpm_chip *chip)
> >diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> >index 1259e935fd58..4e2d00cb0d81 100644
> >--- a/drivers/char/tpm/xen-tpmfront.c
> >+++ b/drivers/char/tpm/xen-tpmfront.c
> >@@ -173,7 +173,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> > return -ETIME;
> > }
> >
> >- return count;
> >+ return 0;
> > }
> >
> > static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> >--
> >2.19.1
> >
>
> Does st33zp24_i2c_send need an update as well? It does 'return write8_reg()'.
Disregard this question. It is st33zp24_send that would be the one to
change, and you already dealt with that.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 0/2] tpm: Unify send() callbacks
2019-02-08 18:08 [PATCH v4 0/2] tpm: Unify send() callbacks Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 2/2] tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete Jarkko Sakkinen
@ 2019-02-09 20:08 ` Jerry Snitselaar
2019-02-11 14:03 ` Jarkko Sakkinen
2 siblings, 1 reply; 20+ messages in thread
From: Jerry Snitselaar @ 2019-02-09 20:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen
On Fri Feb 08 19, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.co m>
>
>A portion of send() callbacks have returned length, in many cases just
>returning back what was given as an argument, and tpm_crb has returned 0 on
>success. This patch set fixes and unifies the behaviour.
>
>v4:
>* Return zero already in tpm_tis_send_main() so that it is applied to all
> call sites of tpm_tis_send().
>* Fixup tpm_ibmvtpm_send() documentation.
>v3:
>tpm_tis_core fix was left out of the staging area :-(
>v2:
>The drivers tpm_nsc and tpm_infineon were forgotten. For this version I
>checked both with find and command and from Kconfig that everything that is
>supposed to be a driver directly interfacing with the TPM core, is included
>(e.g. discluding tpm_tis_spi).
>
>Jarkko Sakkinen (2):
> tpm: Unify the send callback behaviour
> tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete
>
> drivers/char/tpm/st33zp24/st33zp24.c | 2 +-
> drivers/char/tpm/tpm_atmel.c | 2 +-
> drivers/char/tpm/tpm_i2c_atmel.c | 10 +++++++++-
> drivers/char/tpm/tpm_i2c_infineon.c | 2 +-
> drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +-
> drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++----
> drivers/char/tpm/tpm_infineon.c | 2 +-
> drivers/char/tpm/tpm_nsc.c | 2 +-
> drivers/char/tpm/tpm_tis_core.c | 2 +-
> drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
> drivers/char/tpm/xen-tpmfront.c | 2 +-
> 11 files changed, 22 insertions(+), 15 deletions(-)
>
>--
>2.19.1
>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-09 18:20 ` Jerry Snitselaar
2019-02-09 20:01 ` Jerry Snitselaar
@ 2019-02-11 13:58 ` Jarkko Sakkinen
1 sibling, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-11 13:58 UTC (permalink / raw)
To: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen,
stable
On Sat, Feb 09, 2019 at 11:20:22AM -0700, Jerry Snitselaar wrote:
> Does st33zp24_i2c_send need an update as well? It does 'return
> write8_reg()'.
After these commits the only situation when st33zp24_i2c_send() return
value is returned back to the user space, when called from
st33zp24_send(), is when it returns -errno.
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 0/2] tpm: Unify send() callbacks
2019-02-09 20:08 ` [PATCH v4 0/2] tpm: Unify send() callbacks Jerry Snitselaar
@ 2019-02-11 14:03 ` Jarkko Sakkinen
0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-11 14:03 UTC (permalink / raw)
To: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, Alexander Steffen
On Sat, Feb 09, 2019 at 01:08:38PM -0700, Jerry Snitselaar wrote:
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Thank you.
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-08 19:00 ` Jarkko Sakkinen
2019-02-08 19:17 ` [PATCH v4 1/2] tpm: Unify the send callback behaviourä Jarkko Sakkinen
@ 2019-02-11 15:05 ` Alexander Steffen
2019-02-13 7:44 ` Jarkko Sakkinen
1 sibling, 1 reply; 20+ messages in thread
From: Alexander Steffen @ 2019-02-11 15:05 UTC (permalink / raw)
To: Jarkko Sakkinen, Stefan Berger
Cc: linux-integrity, linux-kernel, linux-security-module,
Peter Huewe, Jason Gunthorpe, Stefan Berger, stable
On 08.02.2019 20:00, Jarkko Sakkinen wrote:
> On Fri, Feb 08, 2019 at 01:12:34PM -0500, Stefan Berger wrote:
>> On 2/8/19 1:08 PM, Jarkko Sakkinen wrote:
>>> The send() callback should never return length as it does not in every
>>> driver except tpm_crb in the success case. The reason is that the main
>>> transmit functionality only cares about whether the transmit was
>>> successful or not and ignores the count completely.
>>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Let me know when you put it into your tree, I'll give it a spin while I am
>> at it. :-)
>
> Thank you Stefan! I also add your suggested-by to the first commit
> because you pointed out the problem.
>
> It all looks now legit, but just in case I'll add a check for the return
> value to tpm_try_transmit() and a warning if it is not zero in the
> success case (and after that zeroing of rc).
>
> That check can be removed when I do v5.3 pull request. That should
> enough window to catch any potential issues and check will ensure that
> kernel won't fail even there was something forgotten.
>
> Alexander, I'll push this version now to the master and next with the
> additional check described in this commit, but will add your tags
> after you have time to test.
I ran all tests again and everything works now as expected :)
Tested-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Alexander
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/2] tpm: Unify the send callback behaviour
2019-02-11 15:05 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Alexander Steffen
@ 2019-02-13 7:44 ` Jarkko Sakkinen
0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2019-02-13 7:44 UTC (permalink / raw)
To: Alexander Steffen
Cc: Stefan Berger, linux-integrity, linux-kernel,
linux-security-module, Peter Huewe, Jason Gunthorpe,
Stefan Berger, stable
On Mon, Feb 11, 2019 at 04:05:39PM +0100, Alexander Steffen wrote:
> Tested-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Thank you!
/Jarkko
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-02-13 7:44 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 18:08 [PATCH v4 0/2] tpm: Unify send() callbacks Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Jarkko Sakkinen
2019-02-08 18:12 ` Stefan Berger
2019-02-08 19:00 ` Jarkko Sakkinen
2019-02-08 19:17 ` [PATCH v4 1/2] tpm: Unify the send callback behaviourä Jarkko Sakkinen
2019-02-08 19:27 ` Stefan Berger
2019-02-08 20:23 ` Jarkko Sakkinen
2019-02-08 20:32 ` Stefan Berger
2019-02-08 20:46 ` Jarkko Sakkinen
2019-02-08 21:18 ` Stefan Berger
2019-02-08 21:51 ` Jarkko Sakkinen
2019-02-08 20:38 ` Jarkko Sakkinen
2019-02-11 15:05 ` [PATCH v4 1/2] tpm: Unify the send callback behaviour Alexander Steffen
2019-02-13 7:44 ` Jarkko Sakkinen
2019-02-09 18:20 ` Jerry Snitselaar
2019-02-09 20:01 ` Jerry Snitselaar
2019-02-11 13:58 ` Jarkko Sakkinen
2019-02-08 18:08 ` [PATCH v4 2/2] tpm/tpm_i2c_atmel: Return -E2BIG when the transfer is incomplete Jarkko Sakkinen
2019-02-09 20:08 ` [PATCH v4 0/2] tpm: Unify send() callbacks Jerry Snitselaar
2019-02-11 14:03 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).