Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Tue, May 28, 2019 at 03:02:49PM -0400, Laura Abbott wrote: > > Great, I'll add it. Thank you. Just want to be explicit with these > > things as I consider them as if I was asking a signature from someone > > :-) > > > > /Jarkko > > > Was this intended to go in for 5.2? I still don't see it in the tree. Was intended but I failed to notice that I should start to send PRs to Linus instead of security tree and was waiting for security tree to be rebased. I'll include to the next PR. /Jarkko
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On 4/3/19 1:52 PM, Jarkko Sakkinen wrote: On Tue, Apr 02, 2019 at 07:13:52PM +, Winkler, Tomas wrote: On Tue, Apr 02, 2019 at 02:46:25AM +0300, Jarkko Sakkinen wrote: On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: A "get random" may fail with a TPM error, but those codes were returned as-is to the caller, which assumed the result was the number of bytes that had been written to the target buffer, which could lead to a kernel heap memory exposure and over-read. This fixes tpm1_get_random() to mask positive TPM errors into -EIO, as before. [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random [ 18.092106] usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-64' (offset 0, size 379)! Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 Reported-by: Phil Baker Reported-by: Craig Robson Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using tpm_buf structure") Cc: Laura Abbott Cc: Tomas Winkler Cc: Jarkko Sakkinen Cc: sta...@vger.kernel.org Signed-off-by: Kees Cook --- v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) v2: also fix tpm2 implementation (Jason Gunthorpe) --- drivers/char/tpm/tpm1-cmd.c | 7 +-- drivers/char/tpm/tpm2-cmd.c | 7 +-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index 85dcf2654d11..faacbe1ffa1a 100644 --- a/drivers/char/tpm/tpm1-cmd.c +++ b/drivers/char/tpm/tpm1-cmd.c @@ -510,7 +510,7 @@ struct tpm1_get_random_out { * * Return: * * number of bytes read - * * -errno or a TPM return code otherwise + * * -errno (positive TPM return codes are masked to -EIO) */ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), "attempting get random"); - if (rc) + if (rc) { + if (rc > 0) + rc = -EIO; goto out; + } out = (struct tpm1_get_random_out *)[TPM_HEADER_SIZE]; diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index e74c5b7b64bf..8ffa6af61580 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -301,7 +301,7 @@ struct tpm2_get_random_out { * * Return: * size of the buffer on success, - * -errno otherwise + * -errno otherwise ((positive TPM return codes are masked to -EIO) */ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { @@ -328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) offsetof(struct tpm2_get_random_out, buffer), "attempting get random"); - if (err) + if (err) { + if (err > 0) + err = -EIO; goto out; + } out = (struct tpm2_get_random_out *) [TPM_HEADER_SIZE]; -- 2.17.1 -- Kees Cook Reviewed-by: Jarkko Sakkinen Applied to my master branch. Jason, Tomas, do you want me to add reviewed- by's? Sure, it fixes my patch. Great, I'll add it. Thank you. Just want to be explicit with these things as I consider them as if I was asking a signature from someone :-) /Jarkko Was this intended to go in for 5.2? I still don't see it in the tree. Thanks, Laura
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Tue, Apr 02, 2019 at 07:13:52PM +, Winkler, Tomas wrote: > > > > On Tue, Apr 02, 2019 at 02:46:25AM +0300, Jarkko Sakkinen wrote: > > > On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: > > > > A "get random" may fail with a TPM error, but those codes were > > > > returned as-is to the caller, which assumed the result was the > > > > number of bytes that had been written to the target buffer, which > > > > could lead to a kernel heap memory exposure and over-read. > > > > > > > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, > > > > as before. > > > > > > > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get > > random > > > > [ 18.092106] usercopy: Kernel memory exposure attempt detected from > > SLUB object 'kmalloc-64' (offset 0, size 379)! > > > > > > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > > > > Reported-by: Phil Baker > > > > Reported-by: Craig Robson > > > > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using > > > > tpm_buf structure") > > > > Cc: Laura Abbott > > > > Cc: Tomas Winkler > > > > Cc: Jarkko Sakkinen > > > > Cc: sta...@vger.kernel.org > > > > Signed-off-by: Kees Cook > > > > --- > > > > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > > > > v2: also fix tpm2 implementation (Jason Gunthorpe) > > > > --- > > > > drivers/char/tpm/tpm1-cmd.c | 7 +-- > > > > drivers/char/tpm/tpm2-cmd.c | 7 +-- > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm1-cmd.c > > > > b/drivers/char/tpm/tpm1-cmd.c index 85dcf2654d11..faacbe1ffa1a > > > > 100644 > > > > --- a/drivers/char/tpm/tpm1-cmd.c > > > > +++ b/drivers/char/tpm/tpm1-cmd.c > > > > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > > > > * > > > > * Return: > > > > * * number of bytes read > > > > - * * -errno or a TPM return code otherwise > > > > + * * -errno (positive TPM return codes are masked to -EIO) > > > > */ > > > > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { > > > > @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 > > > > *dest, size_t max) > > > > > > > > rc = tpm_transmit_cmd(chip, , > > > > sizeof(out->rng_data_len), > > > > "attempting get random"); > > > > - if (rc) > > > > + if (rc) { > > > > + if (rc > 0) > > > > + rc = -EIO; > > > > goto out; > > > > + } > > > > > > > > out = (struct tpm1_get_random_out > > *)[TPM_HEADER_SIZE]; > > > > > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c > > > > b/drivers/char/tpm/tpm2-cmd.c index e74c5b7b64bf..8ffa6af61580 > > > > 100644 > > > > --- a/drivers/char/tpm/tpm2-cmd.c > > > > +++ b/drivers/char/tpm/tpm2-cmd.c > > > > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > > > > * > > > > * Return: > > > > * size of the buffer on success, > > > > - * -errno otherwise > > > > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > > > > */ > > > > int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { > > > > @@ -328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 > > *dest, size_t max) > > > >offsetof(struct > > > > tpm2_get_random_out, > > > > buffer), > > > >"attempting get random"); > > > > - if (err) > > > > + if (err) { > > > > + if (err > 0) > > > > + err = -EIO; > > > > goto out; > > > > + } > > > > > > > > out = (struct tpm2_get_random_out *) > > > > [TPM_HEADER_SIZE]; > > > > -- > > > > 2.17.1 > > > > > > > > > > > > -- > > > > Kees Cook > > > > > > Reviewed-by: Jarkko Sakkinen > > > > Applied to my master branch. Jason, Tomas, do you want me to add reviewed- > > by's? > Sure, it fixes my patch. Great, I'll add it. Thank you. Just want to be explicit with these things as I consider them as if I was asking a signature from someone :-) /Jarkko
RE: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
> On Tue, Apr 02, 2019 at 02:46:25AM +0300, Jarkko Sakkinen wrote: > > On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: > > > A "get random" may fail with a TPM error, but those codes were > > > returned as-is to the caller, which assumed the result was the > > > number of bytes that had been written to the target buffer, which > > > could lead to a kernel heap memory exposure and over-read. > > > > > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, > > > as before. > > > > > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get > random > > > [ 18.092106] usercopy: Kernel memory exposure attempt detected from > SLUB object 'kmalloc-64' (offset 0, size 379)! > > > > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > > > Reported-by: Phil Baker > > > Reported-by: Craig Robson > > > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using > > > tpm_buf structure") > > > Cc: Laura Abbott > > > Cc: Tomas Winkler > > > Cc: Jarkko Sakkinen > > > Cc: sta...@vger.kernel.org > > > Signed-off-by: Kees Cook > > > --- > > > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > > > v2: also fix tpm2 implementation (Jason Gunthorpe) > > > --- > > > drivers/char/tpm/tpm1-cmd.c | 7 +-- > > > drivers/char/tpm/tpm2-cmd.c | 7 +-- > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/char/tpm/tpm1-cmd.c > > > b/drivers/char/tpm/tpm1-cmd.c index 85dcf2654d11..faacbe1ffa1a > > > 100644 > > > --- a/drivers/char/tpm/tpm1-cmd.c > > > +++ b/drivers/char/tpm/tpm1-cmd.c > > > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > > > * > > > * Return: > > > * * number of bytes read > > > - * * -errno or a TPM return code otherwise > > > + * * -errno (positive TPM return codes are masked to -EIO) > > > */ > > > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { > > > @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 > > > *dest, size_t max) > > > > > > rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), > > > "attempting get random"); > > > - if (rc) > > > + if (rc) { > > > + if (rc > 0) > > > + rc = -EIO; > > > goto out; > > > + } > > > > > > out = (struct tpm1_get_random_out > *)[TPM_HEADER_SIZE]; > > > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c > > > b/drivers/char/tpm/tpm2-cmd.c index e74c5b7b64bf..8ffa6af61580 > > > 100644 > > > --- a/drivers/char/tpm/tpm2-cmd.c > > > +++ b/drivers/char/tpm/tpm2-cmd.c > > > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > > > * > > > * Return: > > > * size of the buffer on success, > > > - * -errno otherwise > > > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > > > */ > > > int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { > > > @@ -328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 > *dest, size_t max) > > > offsetof(struct tpm2_get_random_out, > > > buffer), > > > "attempting get random"); > > > - if (err) > > > + if (err) { > > > + if (err > 0) > > > + err = -EIO; > > > goto out; > > > + } > > > > > > out = (struct tpm2_get_random_out *) > > > [TPM_HEADER_SIZE]; > > > -- > > > 2.17.1 > > > > > > > > > -- > > > Kees Cook > > > > Reviewed-by: Jarkko Sakkinen > > Applied to my master branch. Jason, Tomas, do you want me to add reviewed- > by's? Sure, it fixes my patch. > /Jarkko
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Tue, Apr 02, 2019 at 02:46:25AM +0300, Jarkko Sakkinen wrote: > On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: > > A "get random" may fail with a TPM error, but those codes were returned > > as-is to the caller, which assumed the result was the number of bytes > > that had been written to the target buffer, which could lead to a kernel > > heap memory exposure and over-read. > > > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, as > > before. > > > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random > > [ 18.092106] usercopy: Kernel memory exposure attempt detected from SLUB > > object 'kmalloc-64' (offset 0, size 379)! > > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > > Reported-by: Phil Baker > > Reported-by: Craig Robson > > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using tpm_buf > > structure") > > Cc: Laura Abbott > > Cc: Tomas Winkler > > Cc: Jarkko Sakkinen > > Cc: sta...@vger.kernel.org > > Signed-off-by: Kees Cook > > --- > > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > > v2: also fix tpm2 implementation (Jason Gunthorpe) > > --- > > drivers/char/tpm/tpm1-cmd.c | 7 +-- > > drivers/char/tpm/tpm2-cmd.c | 7 +-- > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > > index 85dcf2654d11..faacbe1ffa1a 100644 > > --- a/drivers/char/tpm/tpm1-cmd.c > > +++ b/drivers/char/tpm/tpm1-cmd.c > > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > > * > > * Return: > > * * number of bytes read > > - * * -errno or a TPM return code otherwise > > + * * -errno (positive TPM return codes are masked to -EIO) > > */ > > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > > { > > @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, > > size_t max) > > > > rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), > > "attempting get random"); > > - if (rc) > > + if (rc) { > > + if (rc > 0) > > + rc = -EIO; > > goto out; > > + } > > > > out = (struct tpm1_get_random_out *)[TPM_HEADER_SIZE]; > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index e74c5b7b64bf..8ffa6af61580 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > > * > > * Return: > > * size of the buffer on success, > > - * -errno otherwise > > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > > */ > > int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > > { > > @@ -328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, > > size_t max) > >offsetof(struct tpm2_get_random_out, > > buffer), > >"attempting get random"); > > - if (err) > > + if (err) { > > + if (err > 0) > > + err = -EIO; > > goto out; > > + } > > > > out = (struct tpm2_get_random_out *) > > [TPM_HEADER_SIZE]; > > -- > > 2.17.1 > > > > > > -- > > Kees Cook > > Reviewed-by: Jarkko Sakkinen Applied to my master branch. Jason, Tomas, do you want me to add reviewed-by's? /Jarkko
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: > A "get random" may fail with a TPM error, but those codes were returned > as-is to the caller, which assumed the result was the number of bytes > that had been written to the target buffer, which could lead to a kernel > heap memory exposure and over-read. > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, as > before. > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random > [ 18.092106] usercopy: Kernel memory exposure attempt detected from SLUB > object 'kmalloc-64' (offset 0, size 379)! > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > Reported-by: Phil Baker > Reported-by: Craig Robson > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using tpm_buf > structure") > Cc: Laura Abbott > Cc: Tomas Winkler > Cc: Jarkko Sakkinen > Cc: sta...@vger.kernel.org > Signed-off-by: Kees Cook > --- > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > v2: also fix tpm2 implementation (Jason Gunthorpe) > --- > drivers/char/tpm/tpm1-cmd.c | 7 +-- > drivers/char/tpm/tpm2-cmd.c | 7 +-- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > index 85dcf2654d11..faacbe1ffa1a 100644 > --- a/drivers/char/tpm/tpm1-cmd.c > +++ b/drivers/char/tpm/tpm1-cmd.c > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > * > * Return: > * * number of bytes read > - * * -errno or a TPM return code otherwise > + * * -errno (positive TPM return codes are masked to -EIO) > */ > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > { > @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, > size_t max) > > rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), > "attempting get random"); > - if (rc) > + if (rc) { > + if (rc > 0) > + rc = -EIO; > goto out; > + } > > out = (struct tpm1_get_random_out *)[TPM_HEADER_SIZE]; > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index e74c5b7b64bf..8ffa6af61580 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > * > * Return: > * size of the buffer on success, > - * -errno otherwise > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > */ > int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > { > @@ -328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, > size_t max) > offsetof(struct tpm2_get_random_out, > buffer), > "attempting get random"); > - if (err) > + if (err) { > + if (err > 0) > + err = -EIO; > goto out; > + } > > out = (struct tpm2_get_random_out *) > [TPM_HEADER_SIZE]; > -- > 2.17.1 > > > -- > Kees Cook Reviewed-by: Jarkko Sakkinen /Jarkko
RE: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
> > A "get random" may fail with a TPM error, but those codes were returned as-is > to the caller, which assumed the result was the number of bytes that had been > written to the target buffer, which could lead to a kernel heap memory > exposure and over-read. > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, as before. > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random > [ 18.092106] usercopy: Kernel memory exposure attempt detected from SLUB > object 'kmalloc-64' (offset 0, size 379)! > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > Reported-by: Phil Baker > Reported-by: Craig Robson > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using tpm_buf > structure") > Cc: Laura Abbott > Cc: Tomas Winkler > Cc: Jarkko Sakkinen > Cc: sta...@vger.kernel.org > Signed-off-by: Kees Cook > --- > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > v2: also fix tpm2 implementation (Jason Gunthorpe) Looks good to me. Thanks Tomas > --- > drivers/char/tpm/tpm1-cmd.c | 7 +-- drivers/char/tpm/tpm2-cmd.c | 7 > +-- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > index 85dcf2654d11..faacbe1ffa1a 100644 > --- a/drivers/char/tpm/tpm1-cmd.c > +++ b/drivers/char/tpm/tpm1-cmd.c > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > * > * Return: > * * number of bytes read > - * * -errno or a TPM return code otherwise > + * * -errno (positive TPM return codes are masked to -EIO) > */ > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { @@ - > 531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, > size_t max) > > rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), > "attempting get random"); > - if (rc) > + if (rc) { > + if (rc > 0) > + rc = -EIO; > goto out; > + } > > out = (struct tpm1_get_random_out > *)[TPM_HEADER_SIZE]; > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index e74c5b7b64bf..8ffa6af61580 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > * > * Return: > * size of the buffer on success, > - * -errno otherwise > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > */ > int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { @@ - > 328,8 +328,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, > size_t max) > offsetof(struct tpm2_get_random_out, > buffer), > "attempting get random"); > - if (err) > + if (err) { > + if (err > 0) > + err = -EIO; > goto out; > + } > > out = (struct tpm2_get_random_out *) > [TPM_HEADER_SIZE]; > -- > 2.17.1 > > > -- > Kees Cook
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Mon, Apr 1, 2019 at 12:09 PM Jason Gunthorpe wrote: > > + * -errno otherwise ((positive TPM return codes are masked to -EIO) > > Extra bracket, but otherwise looks fine to me Thanks! (And I guess it's time for lunch so I can take a break and learn how to type again...) -- Kees Cook
Re: [PATCH v3] tpm: Actually fail on TPM errors during "get random"
On Mon, Apr 01, 2019 at 12:06:07PM -0700, Kees Cook wrote: > A "get random" may fail with a TPM error, but those codes were returned > as-is to the caller, which assumed the result was the number of bytes > that had been written to the target buffer, which could lead to a kernel > heap memory exposure and over-read. > > This fixes tpm1_get_random() to mask positive TPM errors into -EIO, as > before. > > [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random > [ 18.092106] usercopy: Kernel memory exposure attempt detected from SLUB > object 'kmalloc-64' (offset 0, size 379)! > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1650989 > Reported-by: Phil Baker > Reported-by: Craig Robson > Fixes: 7aee9c52d7ac ("tpm: tpm1: rewrite tpm1_get_random() using tpm_buf > structure") > Cc: Laura Abbott > Cc: Tomas Winkler > Cc: Jarkko Sakkinen > Cc: sta...@vger.kernel.org > Signed-off-by: Kees Cook > v3: fix never-succeed, limit checks to tpm cmd return (James, Jason) > v2: also fix tpm2 implementation (Jason Gunthorpe) > drivers/char/tpm/tpm1-cmd.c | 7 +-- > drivers/char/tpm/tpm2-cmd.c | 7 +-- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > index 85dcf2654d11..faacbe1ffa1a 100644 > +++ b/drivers/char/tpm/tpm1-cmd.c > @@ -510,7 +510,7 @@ struct tpm1_get_random_out { > * > * Return: > * * number of bytes read > - * * -errno or a TPM return code otherwise > + * * -errno (positive TPM return codes are masked to -EIO) > */ > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > { > @@ -531,8 +531,11 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, > size_t max) > > rc = tpm_transmit_cmd(chip, , sizeof(out->rng_data_len), > "attempting get random"); > - if (rc) > + if (rc) { > + if (rc > 0) > + rc = -EIO; > goto out; > + } > > out = (struct tpm1_get_random_out *)[TPM_HEADER_SIZE]; > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index e74c5b7b64bf..8ffa6af61580 100644 > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -301,7 +301,7 @@ struct tpm2_get_random_out { > * > * Return: > * size of the buffer on success, > - * -errno otherwise > + * -errno otherwise ((positive TPM return codes are masked to -EIO) Extra bracket, but otherwise looks fine to me Jason