[PATCH] mmc: cavium: Fix use-after-free in of_platform_device

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
@ 2017-09-07 11:24 Jan Glauber
  2017-09-07 12:07 ` Ulf Hansson
  2017-09-07 16:58 ` Rob Herring
  0 siblings, 2 replies; 9+ messages in thread
From: Jan Glauber @ 2017-09-07 11:24 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: David Daney, linux-mmc, Rob Herring, linux-kernel, Jan Glauber

KASAN reported the following:

[   19.338655] ==================================================================
[   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
[   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264

[   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
[   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
[   19.345995] Call trace:
[   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
[   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
[   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
[   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
[   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
[   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
[   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
[   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
[   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
[   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
[   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
[   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
[   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
[   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
[   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
[   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
[   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
[   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
[   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
[   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
[   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
[   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4

This is caused by:

  platform_device_register()
   -> platform_device_unregister(to_platform_device(dev))
	freeing struct device
   -> of_node_clear_flag(dev->of_node, ...)
	writing to the freed device

The issue is solved by increasing the reference count before calling
of_platform_device_destroy() so freeing the device is postponed after
the call.

Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/mmc/host/cavium-thunderx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
index b9cc95998799..eee08d81b242 100644
--- a/drivers/mmc/host/cavium-thunderx.c
+++ b/drivers/mmc/host/cavium-thunderx.c
@@ -7,6 +7,7 @@
  *
  * Copyright (C) 2016 Cavium Inc.
  */
+#include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/mmc/mmc.h>
@@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
 	for (i = 0; i < CAVIUM_MAX_MMC; i++) {
 		if (host->slot[i])
 			cvm_mmc_of_slot_remove(host->slot[i]);
-		if (host->slot_pdev[i])
+		if (host->slot_pdev[i]) {
+			get_device(&host->slot_pdev[i]->dev);
 			of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
+			put_device(&host->slot_pdev[i]->dev);
+		}
 	}
 	clk_disable_unprepare(host->clk);
 	return ret;
-- 
2.9.0.rc0.21.g7777322

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 11:24 [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy Jan Glauber
@ 2017-09-07 12:07 ` Ulf Hansson
  2017-09-07 12:19   ` Jan Glauber
  2017-09-07 16:58 ` Rob Herring
  1 sibling, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2017-09-07 12:07 UTC (permalink / raw)
  To: Jan Glauber
  Cc: David Daney, linux-mmc@vger.kernel.org, Rob Herring,
	linux-kernel@vger.kernel.org

On 7 September 2017 at 13:24, Jan Glauber <jglauber@cavium.com> wrote:
> KASAN reported the following:
>
> [   19.338655] ==================================================================
> [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
> [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
>
> [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
> [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
> [   19.345995] Call trace:
> [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
> [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
> [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
> [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
> [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
> [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
> [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
> [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
> [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
> [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
> [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
> [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
> [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
> [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
> [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
> [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
> [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
> [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
> [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
> [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
> [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
> [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
>
> This is caused by:
>
>   platform_device_register()
>    -> platform_device_unregister(to_platform_device(dev))
>         freeing struct device
>    -> of_node_clear_flag(dev->of_node, ...)
>         writing to the freed device
>
> The issue is solved by increasing the reference count before calling
> of_platform_device_destroy() so freeing the device is postponed after
> the call.
>
> Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
> Signed-off-by: Jan Glauber <jglauber@cavium.com>

Thanks, applied for fixes and added a stable tag.

Kind regards
Uffe

> ---
>  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
> index b9cc95998799..eee08d81b242 100644
> --- a/drivers/mmc/host/cavium-thunderx.c
> +++ b/drivers/mmc/host/cavium-thunderx.c
> @@ -7,6 +7,7 @@
>   *
>   * Copyright (C) 2016 Cavium Inc.
>   */
> +#include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
>  #include <linux/mmc/mmc.h>
> @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
>         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
>                 if (host->slot[i])
>                         cvm_mmc_of_slot_remove(host->slot[i]);
> -               if (host->slot_pdev[i])
> +               if (host->slot_pdev[i]) {
> +                       get_device(&host->slot_pdev[i]->dev);
>                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
> +                       put_device(&host->slot_pdev[i]->dev);
> +               }
>         }
>         clk_disable_unprepare(host->clk);
>         return ret;
> --
> 2.9.0.rc0.21.g7777322
>

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 12:07 ` Ulf Hansson
@ 2017-09-07 12:19   ` Jan Glauber
  2017-09-07 12:21     ` Ulf Hansson
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Glauber @ 2017-09-07 12:19 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: David Daney, linux-mmc@vger.kernel.org, Rob Herring,
	linux-kernel@vger.kernel.org

Thanks Uffe. The fix would only be required for 4.13, as only with that the
Cavium GPIO driver became available.

--Jan

On Thu, Sep 07, 2017 at 02:07:01PM +0200, Ulf Hansson wrote:
> On 7 September 2017 at 13:24, Jan Glauber <jglauber@cavium.com> wrote:
> > KASAN reported the following:
> >
> > [   19.338655] ==================================================================
> > [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
> > [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
> >
> > [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
> > [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
> > [   19.345995] Call trace:
> > [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
> > [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
> > [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
> > [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
> > [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
> > [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
> > [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
> > [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
> > [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
> > [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
> > [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
> > [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
> > [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
> > [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
> > [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
> > [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
> > [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
> > [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
> > [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
> > [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
> > [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
> > [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
> >
> > This is caused by:
> >
> >   platform_device_register()
> >    -> platform_device_unregister(to_platform_device(dev))
> >         freeing struct device
> >    -> of_node_clear_flag(dev->of_node, ...)
> >         writing to the freed device
> >
> > The issue is solved by increasing the reference count before calling
> > of_platform_device_destroy() so freeing the device is postponed after
> > the call.
> >
> > Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> 
> Thanks, applied for fixes and added a stable tag.
> 
> Kind regards
> Uffe
> 
> > ---
> >  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
> > index b9cc95998799..eee08d81b242 100644
> > --- a/drivers/mmc/host/cavium-thunderx.c
> > +++ b/drivers/mmc/host/cavium-thunderx.c
> > @@ -7,6 +7,7 @@
> >   *
> >   * Copyright (C) 2016 Cavium Inc.
> >   */
> > +#include <linux/device.h>
> >  #include <linux/dma-mapping.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/mmc/mmc.h>
> > @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
> >         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
> >                 if (host->slot[i])
> >                         cvm_mmc_of_slot_remove(host->slot[i]);
> > -               if (host->slot_pdev[i])
> > +               if (host->slot_pdev[i]) {
> > +                       get_device(&host->slot_pdev[i]->dev);
> >                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
> > +                       put_device(&host->slot_pdev[i]->dev);
> > +               }
> >         }
> >         clk_disable_unprepare(host->clk);
> >         return ret;
> > --
> > 2.9.0.rc0.21.g7777322
> >

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 12:19   ` Jan Glauber
@ 2017-09-07 12:21     ` Ulf Hansson
  2017-09-07 12:33       ` Jan Glauber
  0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2017-09-07 12:21 UTC (permalink / raw)
  To: Jan Glauber
  Cc: David Daney, linux-mmc@vger.kernel.org, Rob Herring,
	linux-kernel@vger.kernel.org

On 7 September 2017 at 14:19, Jan Glauber
<jan.glauber@caviumnetworks.com> wrote:
> Thanks Uffe. The fix would only be required for 4.13, as only with that the
> Cavium GPIO driver became available.

Okay, I drop the fixes tag then, because it points to a commit present
in 4.12 as well.

Make sense?

Kind regards
Uffe

>
> --Jan
>
> On Thu, Sep 07, 2017 at 02:07:01PM +0200, Ulf Hansson wrote:
>> On 7 September 2017 at 13:24, Jan Glauber <jglauber@cavium.com> wrote:
>> > KASAN reported the following:
>> >
>> > [   19.338655] ==================================================================
>> > [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
>> > [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
>> >
>> > [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
>> > [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
>> > [   19.345995] Call trace:
>> > [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
>> > [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
>> > [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
>> > [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
>> > [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
>> > [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
>> > [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
>> > [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
>> > [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
>> > [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
>> > [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
>> > [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
>> > [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
>> > [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
>> > [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
>> > [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
>> > [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
>> > [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
>> > [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
>> > [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
>> > [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
>> > [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
>> >
>> > This is caused by:
>> >
>> >   platform_device_register()
>> >    -> platform_device_unregister(to_platform_device(dev))
>> >         freeing struct device
>> >    -> of_node_clear_flag(dev->of_node, ...)
>> >         writing to the freed device
>> >
>> > The issue is solved by increasing the reference count before calling
>> > of_platform_device_destroy() so freeing the device is postponed after
>> > the call.
>> >
>> > Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
>> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>>
>> Thanks, applied for fixes and added a stable tag.
>>
>> Kind regards
>> Uffe
>>
>> > ---
>> >  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
>> > index b9cc95998799..eee08d81b242 100644
>> > --- a/drivers/mmc/host/cavium-thunderx.c
>> > +++ b/drivers/mmc/host/cavium-thunderx.c
>> > @@ -7,6 +7,7 @@
>> >   *
>> >   * Copyright (C) 2016 Cavium Inc.
>> >   */
>> > +#include <linux/device.h>
>> >  #include <linux/dma-mapping.h>
>> >  #include <linux/interrupt.h>
>> >  #include <linux/mmc/mmc.h>
>> > @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
>> >         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
>> >                 if (host->slot[i])
>> >                         cvm_mmc_of_slot_remove(host->slot[i]);
>> > -               if (host->slot_pdev[i])
>> > +               if (host->slot_pdev[i]) {
>> > +                       get_device(&host->slot_pdev[i]->dev);
>> >                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
>> > +                       put_device(&host->slot_pdev[i]->dev);
>> > +               }
>> >         }
>> >         clk_disable_unprepare(host->clk);
>> >         return ret;
>> > --
>> > 2.9.0.rc0.21.g7777322
>> >

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 12:21     ` Ulf Hansson
@ 2017-09-07 12:33       ` Jan Glauber
  2017-09-07 15:00         ` Ulf Hansson
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Glauber @ 2017-09-07 12:33 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: David Daney, linux-mmc@vger.kernel.org, Rob Herring,
	linux-kernel@vger.kernel.org

On Thu, Sep 07, 2017 at 02:21:17PM +0200, Ulf Hansson wrote:
> On 7 September 2017 at 14:19, Jan Glauber
> <jan.glauber@caviumnetworks.com> wrote:
> > Thanks Uffe. The fix would only be required for 4.13, as only with that the
> > Cavium GPIO driver became available.
> 
> Okay, I drop the fixes tag then, because it points to a commit present
> in 4.12 as well.
> 
> Make sense?

Thinking twice it may still be a good idea to have the fixes tag, for
distribution backports. So yes, please keep it.

thanks,
Jan

> Kind regards
> Uffe
> 
> >
> > --Jan
> >
> > On Thu, Sep 07, 2017 at 02:07:01PM +0200, Ulf Hansson wrote:
> >> On 7 September 2017 at 13:24, Jan Glauber <jglauber@cavium.com> wrote:
> >> > KASAN reported the following:
> >> >
> >> > [   19.338655] ==================================================================
> >> > [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
> >> > [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
> >> >
> >> > [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
> >> > [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
> >> > [   19.345995] Call trace:
> >> > [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
> >> > [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
> >> > [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
> >> > [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
> >> > [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
> >> > [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
> >> > [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
> >> > [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
> >> > [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
> >> > [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
> >> > [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
> >> > [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
> >> > [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
> >> > [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
> >> > [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
> >> > [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
> >> > [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
> >> > [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
> >> > [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
> >> > [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
> >> > [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
> >> > [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
> >> >
> >> > This is caused by:
> >> >
> >> >   platform_device_register()
> >> >    -> platform_device_unregister(to_platform_device(dev))
> >> >         freeing struct device
> >> >    -> of_node_clear_flag(dev->of_node, ...)
> >> >         writing to the freed device
> >> >
> >> > The issue is solved by increasing the reference count before calling
> >> > of_platform_device_destroy() so freeing the device is postponed after
> >> > the call.
> >> >
> >> > Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
> >> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> >>
> >> Thanks, applied for fixes and added a stable tag.
> >>
> >> Kind regards
> >> Uffe
> >>
> >> > ---
> >> >  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
> >> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
> >> > index b9cc95998799..eee08d81b242 100644
> >> > --- a/drivers/mmc/host/cavium-thunderx.c
> >> > +++ b/drivers/mmc/host/cavium-thunderx.c
> >> > @@ -7,6 +7,7 @@
> >> >   *
> >> >   * Copyright (C) 2016 Cavium Inc.
> >> >   */
> >> > +#include <linux/device.h>
> >> >  #include <linux/dma-mapping.h>
> >> >  #include <linux/interrupt.h>
> >> >  #include <linux/mmc/mmc.h>
> >> > @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
> >> >         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
> >> >                 if (host->slot[i])
> >> >                         cvm_mmc_of_slot_remove(host->slot[i]);
> >> > -               if (host->slot_pdev[i])
> >> > +               if (host->slot_pdev[i]) {
> >> > +                       get_device(&host->slot_pdev[i]->dev);
> >> >                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
> >> > +                       put_device(&host->slot_pdev[i]->dev);
> >> > +               }
> >> >         }
> >> >         clk_disable_unprepare(host->clk);
> >> >         return ret;
> >> > --
> >> > 2.9.0.rc0.21.g7777322
> >> >

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 12:33       ` Jan Glauber
@ 2017-09-07 15:00         ` Ulf Hansson
  0 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2017-09-07 15:00 UTC (permalink / raw)
  To: Jan Glauber
  Cc: David Daney, linux-mmc@vger.kernel.org, Rob Herring,
	linux-kernel@vger.kernel.org

On 7 September 2017 at 14:33, Jan Glauber
<jan.glauber@caviumnetworks.com> wrote:
> On Thu, Sep 07, 2017 at 02:21:17PM +0200, Ulf Hansson wrote:
>> On 7 September 2017 at 14:19, Jan Glauber
>> <jan.glauber@caviumnetworks.com> wrote:
>> > Thanks Uffe. The fix would only be required for 4.13, as only with that the
>> > Cavium GPIO driver became available.
>>
>> Okay, I drop the fixes tag then, because it points to a commit present
>> in 4.12 as well.
>>
>> Make sense?
>
> Thinking twice it may still be a good idea to have the fixes tag, for
> distribution backports. So yes, please keep it.

Got it, okay!

[...]

Kind regards
Uffe

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 11:24 [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy Jan Glauber
  2017-09-07 12:07 ` Ulf Hansson
@ 2017-09-07 16:58 ` Rob Herring
  2017-09-07 17:15   ` David Daney
  2017-09-07 18:14   ` Jan Glauber
  1 sibling, 2 replies; 9+ messages in thread
From: Rob Herring @ 2017-09-07 16:58 UTC (permalink / raw)
  To: Jan Glauber
  Cc: Ulf Hansson, David Daney, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Thu, Sep 7, 2017 at 6:24 AM, Jan Glauber <jglauber@cavium.com> wrote:
> KASAN reported the following:
>
> [   19.338655] ==================================================================
> [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
> [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
>
> [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
> [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
> [   19.345995] Call trace:
> [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
> [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
> [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
> [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
> [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
> [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
> [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
> [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
> [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
> [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
> [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
> [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
> [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
> [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
> [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
> [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
> [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
> [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
> [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
> [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
> [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
> [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
>
> This is caused by:
>
>   platform_device_register()
>    -> platform_device_unregister(to_platform_device(dev))
>         freeing struct device
>    -> of_node_clear_flag(dev->of_node, ...)
>         writing to the freed device
>
> The issue is solved by increasing the reference count before calling
> of_platform_device_destroy() so freeing the device is postponed after
> the call.
>
> Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
> index b9cc95998799..eee08d81b242 100644
> --- a/drivers/mmc/host/cavium-thunderx.c
> +++ b/drivers/mmc/host/cavium-thunderx.c
> @@ -7,6 +7,7 @@
>   *
>   * Copyright (C) 2016 Cavium Inc.
>   */
> +#include <linux/device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
>  #include <linux/mmc/mmc.h>
> @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
>         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
>                 if (host->slot[i])
>                         cvm_mmc_of_slot_remove(host->slot[i]);
> -               if (host->slot_pdev[i])
> +               if (host->slot_pdev[i]) {
> +                       get_device(&host->slot_pdev[i]->dev);
>                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
> +                       put_device(&host->slot_pdev[i]->dev);

Why do you think this is Cavium specific?

>From my look of it, the problem is in of_platform_device_destroy. We
should save the node ptr before the unregister call and use that to
clear the flags.

Rob

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 16:58 ` Rob Herring
@ 2017-09-07 17:15   ` David Daney
  2017-09-07 18:14   ` Jan Glauber
  1 sibling, 0 replies; 9+ messages in thread
From: David Daney @ 2017-09-07 17:15 UTC (permalink / raw)
  To: Rob Herring, Jan Glauber
  Cc: Ulf Hansson, David Daney, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org

On 09/07/2017 09:58 AM, Rob Herring wrote:
> On Thu, Sep 7, 2017 at 6:24 AM, Jan Glauber <jglauber@cavium.com> wrote:
>> KASAN reported the following:
>>
>> [   19.338655] ==================================================================
>> [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
>> [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
>>
>> [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
>> [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
>> [   19.345995] Call trace:
>> [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
>> [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
>> [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
>> [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
>> [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
>> [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
>> [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
>> [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
>> [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
>> [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
>> [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
>> [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
>> [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
>> [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
>> [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
>> [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
>> [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
>> [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
>> [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
>> [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
>> [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
>> [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
>>
>> This is caused by:
>>
>>    platform_device_register()
>>     -> platform_device_unregister(to_platform_device(dev))
>>          freeing struct device
>>     -> of_node_clear_flag(dev->of_node, ...)
>>          writing to the freed device
>>
>> The issue is solved by increasing the reference count before calling
>> of_platform_device_destroy() so freeing the device is postponed after
>> the call.
>>
>> Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
>> Signed-off-by: Jan Glauber <jglauber@cavium.com>
>> ---
>>   drivers/mmc/host/cavium-thunderx.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
>> index b9cc95998799..eee08d81b242 100644
>> --- a/drivers/mmc/host/cavium-thunderx.c
>> +++ b/drivers/mmc/host/cavium-thunderx.c
>> @@ -7,6 +7,7 @@
>>    *
>>    * Copyright (C) 2016 Cavium Inc.
>>    */
>> +#include <linux/device.h>
>>   #include <linux/dma-mapping.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/mmc/mmc.h>
>> @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
>>          for (i = 0; i < CAVIUM_MAX_MMC; i++) {
>>                  if (host->slot[i])
>>                          cvm_mmc_of_slot_remove(host->slot[i]);
>> -               if (host->slot_pdev[i])
>> +               if (host->slot_pdev[i]) {
>> +                       get_device(&host->slot_pdev[i]->dev);
>>                          of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
>> +                       put_device(&host->slot_pdev[i]->dev);
> 
> Why do you think this is Cavium specific?
> 
>  From my look of it, the problem is in of_platform_device_destroy. We
> should save the node ptr before the unregister call and use that to
> clear the flags.
> 

I agree, forcing all users of the core functions like 
of_platform_device_destroy() to get this right every time is not likely 
to work.

Modifying of_platform_device_destroy() to make it safe to call from this 
context is preferable as it will lead to a more robust kernel overall.

Thanks,
David Daney


> Rob
> 

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

* Re: [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy
  2017-09-07 16:58 ` Rob Herring
  2017-09-07 17:15   ` David Daney
@ 2017-09-07 18:14   ` Jan Glauber
  1 sibling, 0 replies; 9+ messages in thread
From: Jan Glauber @ 2017-09-07 18:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ulf Hansson, David Daney, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Thu, Sep 07, 2017 at 11:58:58AM -0500, Rob Herring wrote:
> On Thu, Sep 7, 2017 at 6:24 AM, Jan Glauber <jglauber@cavium.com> wrote:
> > KASAN reported the following:
> >
> > [   19.338655] ==================================================================
> > [   19.345946] BUG: KASAN: use-after-free in of_platform_device_destroy+0x88/0x100
> > [   19.345966] Read of size 8 at addr fffffe01aa6f1468 by task systemd-udevd/264
> >
> > [   19.345983] CPU: 1 PID: 264 Comm: systemd-udevd Not tainted 4.13.0-jang+ #737
> > [   19.345989] Hardware name: Cavium ThunderX CN81XX board (DT)
> > [   19.345995] Call trace:
> > [   19.346013] [<fffffc800808b1b0>] dump_backtrace+0x0/0x368
> > [   19.346026] [<fffffc800808b6bc>] show_stack+0x24/0x30
> > [   19.346040] [<fffffc8008cbb944>] dump_stack+0xa4/0xc8
> > [   19.346057] [<fffffc80082c2870>] print_address_description+0x68/0x258
> > [   19.346070] [<fffffc80082c2d70>] kasan_report+0x238/0x2f8
> > [   19.346082] [<fffffc80082c14a8>] __asan_load8+0x88/0xb8
> > [   19.346098] [<fffffc8008aacee0>] of_platform_device_destroy+0x88/0x100
> > [   19.346131] [<fffffc8000e02fa4>] thunder_mmc_probe+0x314/0x550 [thunderx_mmc]
> > [   19.346147] [<fffffc800879d560>] pci_device_probe+0x158/0x1f8
> > [   19.346162] [<fffffc800886e53c>] driver_probe_device+0x394/0x5f8
> > [   19.346174] [<fffffc800886e8f4>] __driver_attach+0x154/0x158
> > [   19.346185] [<fffffc800886b12c>] bus_for_each_dev+0xdc/0x140
> > [   19.346196] [<fffffc800886d9f8>] driver_attach+0x38/0x48
> > [   19.346207] [<fffffc800886d148>] bus_add_driver+0x290/0x3c8
> > [   19.346219] [<fffffc800886fc5c>] driver_register+0xbc/0x1a0
> > [   19.346232] [<fffffc800879b78c>] __pci_register_driver+0xc4/0xd8
> > [   19.346260] [<fffffc8000e80024>] thunder_mmc_driver_init+0x24/0x10000 [thunderx_mmc]
> > [   19.346273] [<fffffc8008083a80>] do_one_initcall+0x98/0x1c0
> > [   19.346289] [<fffffc8008177b54>] do_init_module+0xe0/0x2cc
> > [   19.346303] [<fffffc8008175cf0>] load_module+0x3238/0x35c0
> > [   19.346318] [<fffffc8008176438>] SyS_finit_module+0x190/0x1a0
> > [   19.346329] [<fffffc80080834a0>] __sys_trace_return+0x0/0x4
> >
> > This is caused by:
> >
> >   platform_device_register()
> >    -> platform_device_unregister(to_platform_device(dev))
> >         freeing struct device
> >    -> of_node_clear_flag(dev->of_node, ...)
> >         writing to the freed device
> >
> > The issue is solved by increasing the reference count before calling
> > of_platform_device_destroy() so freeing the device is postponed after
> > the call.
> >
> > Fixes: 8fb83b142823 ("mmc: cavium: Fix probing race with regulator")
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  drivers/mmc/host/cavium-thunderx.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/cavium-thunderx.c b/drivers/mmc/host/cavium-thunderx.c
> > index b9cc95998799..eee08d81b242 100644
> > --- a/drivers/mmc/host/cavium-thunderx.c
> > +++ b/drivers/mmc/host/cavium-thunderx.c
> > @@ -7,6 +7,7 @@
> >   *
> >   * Copyright (C) 2016 Cavium Inc.
> >   */
> > +#include <linux/device.h>
> >  #include <linux/dma-mapping.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/mmc/mmc.h>
> > @@ -149,8 +150,11 @@ static int thunder_mmc_probe(struct pci_dev *pdev,
> >         for (i = 0; i < CAVIUM_MAX_MMC; i++) {
> >                 if (host->slot[i])
> >                         cvm_mmc_of_slot_remove(host->slot[i]);
> > -               if (host->slot_pdev[i])
> > +               if (host->slot_pdev[i]) {
> > +                       get_device(&host->slot_pdev[i]->dev);
> >                         of_platform_device_destroy(&host->slot_pdev[i]->dev, NULL);
> > +                       put_device(&host->slot_pdev[i]->dev);
> 
> Why do you think this is Cavium specific?

First, the usage in the Cavium driver is quite special. The device
is not a platform device, I create this as a dummy device because
mmc_parse_of would not work otherwise.

Second, I assumed the of_node_clear_flag() after removal of the device
is a more general pattern and only not working for our case because
of the dummy device hack. I've not looked too closely at the platform
code, so I might be wrong here.

> >From my look of it, the problem is in of_platform_device_destroy. We
> should save the node ptr before the unregister call and use that to
> clear the flags.

This would be a larger patch and would need much more testing I guess.

--Jan

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

end of thread, other threads:[~2017-09-07 18:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-07 11:24 [PATCH] mmc: cavium: Fix use-after-free in of_platform_device_destroy Jan Glauber
2017-09-07 12:07 ` Ulf Hansson
2017-09-07 12:19   ` Jan Glauber
2017-09-07 12:21     ` Ulf Hansson
2017-09-07 12:33       ` Jan Glauber
2017-09-07 15:00         ` Ulf Hansson
2017-09-07 16:58 ` Rob Herring
2017-09-07 17:15   ` David Daney
2017-09-07 18:14   ` Jan Glauber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox