linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm: Avoid function type cast of put_device()
@ 2022-10-21 12:33 Ard Biesheuvel
  2022-10-21 14:26 ` Jason Gunthorpe
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-10-21 12:33 UTC (permalink / raw)
  To: peterhuewe
  Cc: jarkko, linux-integrity, jgg, keescook, samitolvanen,
	Ard Biesheuvel

The TPM code registers put_device() as a devm cleanup handler, and casts
the reference to the right function pointer type for this to be
permitted by the compiler.

However, under kCFI, this is rejected at runtime, resulting in a splat
like

   CFI failure at devm_action_release+0x24/0x3c (target: put_device+0x0/0x24; expected type: 0xa488ebfc)
   Internal error: Oops - CFI: 0000000000000000 [#1] PREEMPT SMP
   Modules linked in:  ...
   CPU: 20 PID: 454 Comm: systemd-udevd Not tainted 6.1.0-rc1+ #51
   Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #1 Oct  3 2022
   pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
   pc : devm_action_release+0x24/0x3c
   lr : devres_release_all+0xb4/0x114
   sp : ffff800009bb3630
   x29: ffff800009bb3630 x28: 0000000000000000 x27: 0000000000000011
   x26: ffffaa6f9922c0c8 x25: 0000000000000002 x24: 000000000000000f
   x23: ffff800009bb3648 x22: ffff7aefc3be2100 x21: ffff7aefc3be2e00
   x20: 0000000000000005 x19: ffff7aefc1e1ec10 x18: ffff800009af70a8
   x17: 00000000a488ebfc x16: 0000000094ee7df3 x15: 0000000000000000
   x14: 4075c5c2ef7affff x13: e46a91c5c5e2ef42 x12: ffff7aefc2c57540
   x11: 0000000000000001 x10: 0000000000000001 x9 : 0000000100000000
   x8 : ffffaa6fa09b39b4 x7 : 7f7f7f7f7f7f7f7f x6 : 8000000000000000
   x5 : 000000008020000e x4 : ffff7aefc2c57500 x3 : ffff800009bb3648
   x2 : ffff800009bb3648 x1 : ffff7aefc3be2e80 x0 : ffff7aefc3bb7000
   Call trace:
    devm_action_release+0x24/0x3c
    devres_release_all+0xb4/0x114
    really_probe+0xb0/0x49c
    __driver_probe_device+0x114/0x180
    driver_probe_device+0x48/0x1ec
    __driver_attach+0x118/0x284
    bus_for_each_dev+0x94/0xe4
    driver_attach+0x24/0x34
    bus_add_driver+0x10c/0x220
    driver_register+0x78/0x118
    __platform_driver_register+0x24/0x34
    init_module+0x20/0xfe4 [tpm_tis_synquacer]
    do_one_initcall+0xd4/0x248
    do_init_module+0x44/0x28c
    load_module+0x16b4/0x1920

Fix this by going through a helper function of the correct type.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/char/tpm/tpm-chip.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 783d65fc71f0..741d8f3e8fb3 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -373,6 +373,11 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 }
 EXPORT_SYMBOL_GPL(tpm_chip_alloc);
 
+static void tpm_put_device(void *dev)
+{
+	put_device(dev);
+}
+
 /**
  * tpmm_chip_alloc() - allocate a new struct tpm_chip instance
  * @pdev: parent device to which the chip is associated
@@ -391,7 +396,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
 		return chip;
 
 	rc = devm_add_action_or_reset(pdev,
-				      (void (*)(void *)) put_device,
+				      tpm_put_device,
 				      &chip->dev);
 	if (rc)
 		return ERR_PTR(rc);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-21 12:33 [PATCH] tpm: Avoid function type cast of put_device() Ard Biesheuvel
@ 2022-10-21 14:26 ` Jason Gunthorpe
  2022-10-21 16:17 ` Kees Cook
  2022-10-23 21:27 ` Jarkko Sakkinen
  2 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2022-10-21 14:26 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: peterhuewe, jarkko, linux-integrity, keescook, samitolvanen

On Fri, Oct 21, 2022 at 02:33:09PM +0200, Ard Biesheuvel wrote:
> The TPM code registers put_device() as a devm cleanup handler, and casts
> the reference to the right function pointer type for this to be
> permitted by the compiler.
> 
> However, under kCFI, this is rejected at runtime, resulting in a splat
> like
> 
>    CFI failure at devm_action_release+0x24/0x3c (target: put_device+0x0/0x24; expected type: 0xa488ebfc)
>    Internal error: Oops - CFI: 0000000000000000 [#1] PREEMPT SMP
>    Modules linked in:  ...
>    CPU: 20 PID: 454 Comm: systemd-udevd Not tainted 6.1.0-rc1+ #51
>    Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #1 Oct  3 2022
>    pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>    pc : devm_action_release+0x24/0x3c
>    lr : devres_release_all+0xb4/0x114
>    sp : ffff800009bb3630
>    x29: ffff800009bb3630 x28: 0000000000000000 x27: 0000000000000011
>    x26: ffffaa6f9922c0c8 x25: 0000000000000002 x24: 000000000000000f
>    x23: ffff800009bb3648 x22: ffff7aefc3be2100 x21: ffff7aefc3be2e00
>    x20: 0000000000000005 x19: ffff7aefc1e1ec10 x18: ffff800009af70a8
>    x17: 00000000a488ebfc x16: 0000000094ee7df3 x15: 0000000000000000
>    x14: 4075c5c2ef7affff x13: e46a91c5c5e2ef42 x12: ffff7aefc2c57540
>    x11: 0000000000000001 x10: 0000000000000001 x9 : 0000000100000000
>    x8 : ffffaa6fa09b39b4 x7 : 7f7f7f7f7f7f7f7f x6 : 8000000000000000
>    x5 : 000000008020000e x4 : ffff7aefc2c57500 x3 : ffff800009bb3648
>    x2 : ffff800009bb3648 x1 : ffff7aefc3be2e80 x0 : ffff7aefc3bb7000
>    Call trace:
>     devm_action_release+0x24/0x3c
>     devres_release_all+0xb4/0x114
>     really_probe+0xb0/0x49c
>     __driver_probe_device+0x114/0x180
>     driver_probe_device+0x48/0x1ec
>     __driver_attach+0x118/0x284
>     bus_for_each_dev+0x94/0xe4
>     driver_attach+0x24/0x34
>     bus_add_driver+0x10c/0x220
>     driver_register+0x78/0x118
>     __platform_driver_register+0x24/0x34
>     init_module+0x20/0xfe4 [tpm_tis_synquacer]
>     do_one_initcall+0xd4/0x248
>     do_init_module+0x44/0x28c
>     load_module+0x16b4/0x1920
> 
> Fix this by going through a helper function of the correct type.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/char/tpm/tpm-chip.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-21 12:33 [PATCH] tpm: Avoid function type cast of put_device() Ard Biesheuvel
  2022-10-21 14:26 ` Jason Gunthorpe
@ 2022-10-21 16:17 ` Kees Cook
  2022-10-23 21:27 ` Jarkko Sakkinen
  2 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2022-10-21 16:17 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: peterhuewe, jarkko, linux-integrity, jgg, samitolvanen

On Fri, Oct 21, 2022 at 02:33:09PM +0200, Ard Biesheuvel wrote:
> The TPM code registers put_device() as a devm cleanup handler, and casts
> the reference to the right function pointer type for this to be
> permitted by the compiler.
> 
> However, under kCFI, this is rejected at runtime, resulting in a splat
> like
> 
>    CFI failure at devm_action_release+0x24/0x3c (target: put_device+0x0/0x24; expected type: 0xa488ebfc)
>    Internal error: Oops - CFI: 0000000000000000 [#1] PREEMPT SMP
>    Modules linked in:  ...
>    CPU: 20 PID: 454 Comm: systemd-udevd Not tainted 6.1.0-rc1+ #51
>    Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #1 Oct  3 2022
>    pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>    pc : devm_action_release+0x24/0x3c
>    lr : devres_release_all+0xb4/0x114
>    sp : ffff800009bb3630
>    x29: ffff800009bb3630 x28: 0000000000000000 x27: 0000000000000011
>    x26: ffffaa6f9922c0c8 x25: 0000000000000002 x24: 000000000000000f
>    x23: ffff800009bb3648 x22: ffff7aefc3be2100 x21: ffff7aefc3be2e00
>    x20: 0000000000000005 x19: ffff7aefc1e1ec10 x18: ffff800009af70a8
>    x17: 00000000a488ebfc x16: 0000000094ee7df3 x15: 0000000000000000
>    x14: 4075c5c2ef7affff x13: e46a91c5c5e2ef42 x12: ffff7aefc2c57540
>    x11: 0000000000000001 x10: 0000000000000001 x9 : 0000000100000000
>    x8 : ffffaa6fa09b39b4 x7 : 7f7f7f7f7f7f7f7f x6 : 8000000000000000
>    x5 : 000000008020000e x4 : ffff7aefc2c57500 x3 : ffff800009bb3648
>    x2 : ffff800009bb3648 x1 : ffff7aefc3be2e80 x0 : ffff7aefc3bb7000
>    Call trace:
>     devm_action_release+0x24/0x3c
>     devres_release_all+0xb4/0x114
>     really_probe+0xb0/0x49c
>     __driver_probe_device+0x114/0x180
>     driver_probe_device+0x48/0x1ec
>     __driver_attach+0x118/0x284
>     bus_for_each_dev+0x94/0xe4
>     driver_attach+0x24/0x34
>     bus_add_driver+0x10c/0x220
>     driver_register+0x78/0x118
>     __platform_driver_register+0x24/0x34
>     init_module+0x20/0xfe4 [tpm_tis_synquacer]
>     do_one_initcall+0xd4/0x248
>     do_init_module+0x44/0x28c
>     load_module+0x16b4/0x1920
> 
> Fix this by going through a helper function of the correct type.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-21 12:33 [PATCH] tpm: Avoid function type cast of put_device() Ard Biesheuvel
  2022-10-21 14:26 ` Jason Gunthorpe
  2022-10-21 16:17 ` Kees Cook
@ 2022-10-23 21:27 ` Jarkko Sakkinen
  2022-10-28 21:08   ` Kees Cook
  2 siblings, 1 reply; 8+ messages in thread
From: Jarkko Sakkinen @ 2022-10-23 21:27 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: peterhuewe, linux-integrity, jgg, keescook, samitolvanen

On Fri, Oct 21, 2022 at 02:33:09PM +0200, Ard Biesheuvel wrote:
> The TPM code registers put_device() as a devm cleanup handler, and casts
> the reference to the right function pointer type for this to be
> permitted by the compiler.
> 
> However, under kCFI, this is rejected at runtime, resulting in a splat
> like
> 
>    CFI failure at devm_action_release+0x24/0x3c (target: put_device+0x0/0x24; expected type: 0xa488ebfc)
>    Internal error: Oops - CFI: 0000000000000000 [#1] PREEMPT SMP
>    Modules linked in:  ...
>    CPU: 20 PID: 454 Comm: systemd-udevd Not tainted 6.1.0-rc1+ #51
>    Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #1 Oct  3 2022
>    pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>    pc : devm_action_release+0x24/0x3c
>    lr : devres_release_all+0xb4/0x114
>    sp : ffff800009bb3630
>    x29: ffff800009bb3630 x28: 0000000000000000 x27: 0000000000000011
>    x26: ffffaa6f9922c0c8 x25: 0000000000000002 x24: 000000000000000f
>    x23: ffff800009bb3648 x22: ffff7aefc3be2100 x21: ffff7aefc3be2e00
>    x20: 0000000000000005 x19: ffff7aefc1e1ec10 x18: ffff800009af70a8
>    x17: 00000000a488ebfc x16: 0000000094ee7df3 x15: 0000000000000000
>    x14: 4075c5c2ef7affff x13: e46a91c5c5e2ef42 x12: ffff7aefc2c57540
>    x11: 0000000000000001 x10: 0000000000000001 x9 : 0000000100000000
>    x8 : ffffaa6fa09b39b4 x7 : 7f7f7f7f7f7f7f7f x6 : 8000000000000000
>    x5 : 000000008020000e x4 : ffff7aefc2c57500 x3 : ffff800009bb3648
>    x2 : ffff800009bb3648 x1 : ffff7aefc3be2e80 x0 : ffff7aefc3bb7000
>    Call trace:
>     devm_action_release+0x24/0x3c
>     devres_release_all+0xb4/0x114
>     really_probe+0xb0/0x49c
>     __driver_probe_device+0x114/0x180
>     driver_probe_device+0x48/0x1ec
>     __driver_attach+0x118/0x284
>     bus_for_each_dev+0x94/0xe4
>     driver_attach+0x24/0x34
>     bus_add_driver+0x10c/0x220
>     driver_register+0x78/0x118
>     __platform_driver_register+0x24/0x34
>     init_module+0x20/0xfe4 [tpm_tis_synquacer]
>     do_one_initcall+0xd4/0x248
>     do_init_module+0x44/0x28c
>     load_module+0x16b4/0x1920
> 
> Fix this by going through a helper function of the correct type.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/char/tpm/tpm-chip.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 783d65fc71f0..741d8f3e8fb3 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -373,6 +373,11 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  }
>  EXPORT_SYMBOL_GPL(tpm_chip_alloc);
>  
> +static void tpm_put_device(void *dev)
> +{
> +	put_device(dev);
> +}
> +
>  /**
>   * tpmm_chip_alloc() - allocate a new struct tpm_chip instance
>   * @pdev: parent device to which the chip is associated
> @@ -391,7 +396,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
>  		return chip;
>  
>  	rc = devm_add_action_or_reset(pdev,
> -				      (void (*)(void *)) put_device,
> +				      tpm_put_device,
>  				      &chip->dev);
>  	if (rc)
>  		return ERR_PTR(rc);
> -- 
> 2.35.1
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-23 21:27 ` Jarkko Sakkinen
@ 2022-10-28 21:08   ` Kees Cook
  2022-10-30  9:23     ` Ard Biesheuvel
  2022-11-01  1:23     ` Jarkko Sakkinen
  0 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2022-10-28 21:08 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Ard Biesheuvel, peterhuewe, linux-integrity, jgg, samitolvanen

On Mon, Oct 24, 2022 at 12:27:16AM +0300, Jarkko Sakkinen wrote:
> [...]
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Who's tree can this land in?

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-28 21:08   ` Kees Cook
@ 2022-10-30  9:23     ` Ard Biesheuvel
  2022-11-01  1:26       ` Jarkko Sakkinen
  2022-11-01  1:23     ` Jarkko Sakkinen
  1 sibling, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2022-10-30  9:23 UTC (permalink / raw)
  To: Kees Cook; +Cc: Jarkko Sakkinen, peterhuewe, linux-integrity, jgg, samitolvanen

On Fri, 28 Oct 2022 at 23:08, Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Oct 24, 2022 at 12:27:16AM +0300, Jarkko Sakkinen wrote:
> > [...]
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Who's tree can this land in?
>

I'd expect the TPM maintainers to take this through the TPM tree.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-28 21:08   ` Kees Cook
  2022-10-30  9:23     ` Ard Biesheuvel
@ 2022-11-01  1:23     ` Jarkko Sakkinen
  1 sibling, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2022-11-01  1:23 UTC (permalink / raw)
  To: Kees Cook; +Cc: Ard Biesheuvel, peterhuewe, linux-integrity, jgg, samitolvanen

On Fri, Oct 28, 2022 at 02:08:24PM -0700, Kees Cook wrote:
> On Mon, Oct 24, 2022 at 12:27:16AM +0300, Jarkko Sakkinen wrote:
> > [...]
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> Who's tree can this land in?

Sorry, I forgot to pick this, it's now applied to my tree.

> 
> -- 
> Kees Cook

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Avoid function type cast of put_device()
  2022-10-30  9:23     ` Ard Biesheuvel
@ 2022-11-01  1:26       ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2022-11-01  1:26 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Kees Cook, peterhuewe, linux-integrity, jgg, samitolvanen

On Sun, Oct 30, 2022 at 10:23:27AM +0100, Ard Biesheuvel wrote:
> On Fri, 28 Oct 2022 at 23:08, Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Oct 24, 2022 at 12:27:16AM +0300, Jarkko Sakkinen wrote:
> > > [...]
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> >
> > Who's tree can this land in?
> >
> 
> I'd expect the TPM maintainers to take this through the TPM tree.

It is now in:

git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/ 

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-01  1:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-21 12:33 [PATCH] tpm: Avoid function type cast of put_device() Ard Biesheuvel
2022-10-21 14:26 ` Jason Gunthorpe
2022-10-21 16:17 ` Kees Cook
2022-10-23 21:27 ` Jarkko Sakkinen
2022-10-28 21:08   ` Kees Cook
2022-10-30  9:23     ` Ard Biesheuvel
2022-11-01  1:26       ` Jarkko Sakkinen
2022-11-01  1:23     ` 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).