[PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Venkata Narendra Kumar Gutta]

Platform core is using pdev->name as the platform device name to do
the binding of the devices with the drivers. But, when the platform
driver overrides the platform device name with dev_set_name(),
the pdev->name is pointing to a location which is freed and becomes
an invalid parameter to do the binding match.

use-after-free instance:

[   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
[   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
[   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
			G S      W  O      4.19.30+ #3
[   33.346835] Hardware name: MTP (DT)
[   33.350419] Call trace:
[   33.352941]  dump_backtrace+0x0/0x3b8
[   33.356713]  show_stack+0x24/0x30
[   33.360119]  dump_stack+0x160/0x1d8
[   33.363709]  print_address_description+0x84/0x2e0
[   33.368549]  kasan_report+0x26c/0x2d0
[   33.372322]  __asan_report_load1_noabort+0x2c/0x38
[   33.377248]  strcmp+0x8c/0xb0
[   33.380306]  platform_match+0x70/0x1f8
[   33.384168]  __driver_attach+0x78/0x3a0
[   33.388111]  bus_for_each_dev+0x13c/0x1b8
[   33.392237]  driver_attach+0x4c/0x58
[   33.395910]  bus_add_driver+0x350/0x560
[   33.399854]  driver_register+0x23c/0x328
[   33.403886]  __platform_driver_register+0xd0/0xe0

So, use dev_name(&pdev->dev), which fetches the platform device name from
the kobject(dev->kobj->name) of the device instead of the pdev->name.

Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
---
 drivers/base/platform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index dab0a5a..0e23aa2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 	if (len != -ENODEV)
 		return len;
 
-	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
+	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
 
 	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
 }
@@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 		return rc;
 
 	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
-			pdev->name);
+			dev_name(&pdev->dev));
 	return 0;
 }
 
@@ -973,7 +973,7 @@ static const struct platform_device_id *platform_match_id(
 			struct platform_device *pdev)
 {
 	while (id->name[0]) {
-		if (strcmp(pdev->name, id->name) == 0) {
+		if (strcmp(dev_name(&pdev->dev), id->name) == 0) {
 			pdev->id_entry = id;
 			return id;
 		}
@@ -1017,7 +1017,7 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 		return platform_match_id(pdrv->id_table, pdev) != NULL;
 
 	/* fall-back to driver name match */
-	return (strcmp(pdev->name, drv->name) == 0);
+	return (strcmp(dev_name(&pdev->dev), drv->name) == 0);
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Krzysztof Kozlowski]

On Tue, 23 Apr 2019 at 05:36, Venkata Narendra Kumar Gutta
<vnkgutta@codeaurora.org> wrote:
>
> Platform core is using pdev->name as the platform device name to do
> the binding of the devices with the drivers. But, when the platform
> driver overrides the platform device name with dev_set_name(),
> the pdev->name is pointing to a location which is freed and becomes

If pdev->name is invalid then it should be removed/fixed. Why leaving
it pointing to wrong place and changing the users to something else?
This looks like either duct-tape for real problem.

> an invalid parameter to do the binding match.
>
> use-after-free instance:
>
> [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
>                         G S      W  O      4.19.30+ #3
> [   33.346835] Hardware name: MTP (DT)
> [   33.350419] Call trace:
> [   33.352941]  dump_backtrace+0x0/0x3b8
> [   33.356713]  show_stack+0x24/0x30
> [   33.360119]  dump_stack+0x160/0x1d8
> [   33.363709]  print_address_description+0x84/0x2e0
> [   33.368549]  kasan_report+0x26c/0x2d0
> [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> [   33.377248]  strcmp+0x8c/0xb0
> [   33.380306]  platform_match+0x70/0x1f8
> [   33.384168]  __driver_attach+0x78/0x3a0
> [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> [   33.392237]  driver_attach+0x4c/0x58
> [   33.395910]  bus_add_driver+0x350/0x560
> [   33.399854]  driver_register+0x23c/0x328
> [   33.403886]  __platform_driver_register+0xd0/0xe0
>
> So, use dev_name(&pdev->dev), which fetches the platform device name from
> the kobject(dev->kobj->name) of the device instead of the pdev->name.
>
> Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> ---
>  drivers/base/platform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dab0a5a..0e23aa2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
>         if (len != -ENODEV)
>                 return len;
>
> -       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> +       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
>
>         return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
>  }
> @@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>                 return rc;
>
>         add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> -                       pdev->name);
> +                       dev_name(&pdev->dev));

This is wrong fix and it causes ARM Vexpress board fail to boot under
QEMU (but probably in real world as well). The problem is in not
mached drivers. For example the pdev->name is "vexpress-syscfg" and
dev_name(&pdev->dev) is "vexpress-syscfg.6.auto". The effect - none of
AMBA devices are registered, including missing MMC device (mmci.c,
arm,pl180).

One can see the error of missing root device:
[   13.458982] VFS: Cannot open root device "mmcblk0" or
unknown-block(0,0): error -6

... also before there is a warning like:
[    0.285029] ------------[ cut here ]------------
[    0.285507] WARNING: CPU: 0 PID: 1 at
/home/krzk/dev/yocto-proceq/build/workspace/sources/linux-mainline-next/drivers/bus/vexpress-config.c:183
vexpress_config_init+0x90/0xe0
[    0.285936] Modules linked in:
[    0.286251] CPU: 0 PID: 1 Comm: swapper Tainted: G        W
5.1.0-rc6-next-20190429-g0593ae1f5df5 #27
[    0.286507] Hardware name: ARM-Versatile Express
[    0.286977] [<8010e05c>] (unwind_backtrace) from [<8010b76c>]
(show_stack+0x10/0x14)
[    0.287219] [<8010b76c>] (show_stack) from [<8011ac64>] (__warn+0xf8/0x110)
[    0.287400] [<8011ac64>] (__warn) from [<8011ad94>]
(warn_slowpath_null+0x40/0x48)
[    0.287579] [<8011ad94>] (warn_slowpath_null) from [<809151bc>]
(vexpress_config_init+0x90/0xe0)
[    0.287811] [<809151bc>] (vexpress_config_init) from [<80102710>]
(do_one_initcall+0x54/0x1b4)
[    0.288014] [<80102710>] (do_one_initcall) from [<80900e84>]
(kernel_init_freeable+0x12c/0x1c8)
[    0.288214] [<80900e84>] (kernel_init_freeable) from [<80634048>]
(kernel_init+0x8/0x110)
[    0.288388] [<80634048>] (kernel_init) from [<801010e8>]
(ret_from_fork+0x14/0x2c)
[    0.288597] Exception stack(0x86835fb0 to 0x86835ff8)
[    0.288882] 5fa0:                                     00000000
00000000 00000000 00000000
[    0.289193] 5fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    0.289479] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.289776] ---[ end trace 3f0995a2bae83983 ]---

Best regards,
Krzysztof

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Krzysztof Kozlowski]

On Tue, 23 Apr 2019 at 05:36, Venkata Narendra Kumar Gutta
<vnkgutta@codeaurora.org> wrote:
>
> Platform core is using pdev->name as the platform device name to do
> the binding of the devices with the drivers. But, when the platform
> driver overrides the platform device name with dev_set_name(),
> the pdev->name is pointing to a location which is freed and becomes

If pdev->name is invalid then it should be removed/fixed. Why leaving
it pointing to wrong place and changing the users to something else?
This looks like either duct-tape for real problem.

> an invalid parameter to do the binding match.
>
> use-after-free instance:
>
> [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
>                         G S      W  O      4.19.30+ #3
> [   33.346835] Hardware name: MTP (DT)
> [   33.350419] Call trace:
> [   33.352941]  dump_backtrace+0x0/0x3b8
> [   33.356713]  show_stack+0x24/0x30
> [   33.360119]  dump_stack+0x160/0x1d8
> [   33.363709]  print_address_description+0x84/0x2e0
> [   33.368549]  kasan_report+0x26c/0x2d0
> [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> [   33.377248]  strcmp+0x8c/0xb0
> [   33.380306]  platform_match+0x70/0x1f8
> [   33.384168]  __driver_attach+0x78/0x3a0
> [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> [   33.392237]  driver_attach+0x4c/0x58
> [   33.395910]  bus_add_driver+0x350/0x560
> [   33.399854]  driver_register+0x23c/0x328
> [   33.403886]  __platform_driver_register+0xd0/0xe0
>
> So, use dev_name(&pdev->dev), which fetches the platform device name from
> the kobject(dev->kobj->name) of the device instead of the pdev->name.
>
> Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> ---
>  drivers/base/platform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dab0a5a..0e23aa2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
>         if (len != -ENODEV)
>                 return len;
>
> -       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> +       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
>
>         return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
>  }
> @@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>                 return rc;
>
>         add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> -                       pdev->name);
> +                       dev_name(&pdev->dev));

This is wrong fix and it causes ARM Vexpress board fail to boot under
QEMU (but probably in real world as well). The problem is in not
mached drivers. For example the pdev->name is "vexpress-syscfg" and
dev_name(&pdev->dev) is "vexpress-syscfg.6.auto". The effect - none of
AMBA devices are registered, including missing MMC device (mmci.c,
arm,pl180).

One can see the error of missing root device:
[   13.458982] VFS: Cannot open root device "mmcblk0" or
unknown-block(0,0): error -6

... also before there is a warning like:
[    0.285029] ------------[ cut here ]------------
[    0.285507] WARNING: CPU: 0 PID: 1 at
/home/krzk/dev/yocto-proceq/build/workspace/sources/linux-mainline-next/drivers/bus/vexpress-config.c:183
vexpress_config_init+0x90/0xe0
[    0.285936] Modules linked in:
[    0.286251] CPU: 0 PID: 1 Comm: swapper Tainted: G        W
5.1.0-rc6-next-20190429-g0593ae1f5df5 #27
[    0.286507] Hardware name: ARM-Versatile Express
[    0.286977] [<8010e05c>] (unwind_backtrace) from [<8010b76c>]
(show_stack+0x10/0x14)
[    0.287219] [<8010b76c>] (show_stack) from [<8011ac64>] (__warn+0xf8/0x110)
[    0.287400] [<8011ac64>] (__warn) from [<8011ad94>]
(warn_slowpath_null+0x40/0x48)
[    0.287579] [<8011ad94>] (warn_slowpath_null) from [<809151bc>]
(vexpress_config_init+0x90/0xe0)
[    0.287811] [<809151bc>] (vexpress_config_init) from [<80102710>]
(do_one_initcall+0x54/0x1b4)
[    0.288014] [<80102710>] (do_one_initcall) from [<80900e84>]
(kernel_init_freeable+0x12c/0x1c8)
[    0.288214] [<80900e84>] (kernel_init_freeable) from [<80634048>]
(kernel_init+0x8/0x110)
[    0.288388] [<80634048>] (kernel_init) from [<801010e8>]
(ret_from_fork+0x14/0x2c)
[    0.288597] Exception stack(0x86835fb0 to 0x86835ff8)
[    0.288882] 5fa0:                                     00000000
00000000 00000000 00000000
[    0.289193] 5fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    0.289479] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.289776] ---[ end trace 3f0995a2bae83983 ]---

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Greg KH]

On Mon, Apr 29, 2019 at 05:07:56PM +0200, Krzysztof Kozlowski wrote:
> On Tue, 23 Apr 2019 at 05:36, Venkata Narendra Kumar Gutta
> <vnkgutta@codeaurora.org> wrote:
> >
> > Platform core is using pdev->name as the platform device name to do
> > the binding of the devices with the drivers. But, when the platform
> > driver overrides the platform device name with dev_set_name(),
> > the pdev->name is pointing to a location which is freed and becomes
> 
> If pdev->name is invalid then it should be removed/fixed. Why leaving
> it pointing to wrong place and changing the users to something else?
> This looks like either duct-tape for real problem.
> 
> > an invalid parameter to do the binding match.
> >
> > use-after-free instance:
> >
> > [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> > [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> > [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
> >                         G S      W  O      4.19.30+ #3
> > [   33.346835] Hardware name: MTP (DT)
> > [   33.350419] Call trace:
> > [   33.352941]  dump_backtrace+0x0/0x3b8
> > [   33.356713]  show_stack+0x24/0x30
> > [   33.360119]  dump_stack+0x160/0x1d8
> > [   33.363709]  print_address_description+0x84/0x2e0
> > [   33.368549]  kasan_report+0x26c/0x2d0
> > [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> > [   33.377248]  strcmp+0x8c/0xb0
> > [   33.380306]  platform_match+0x70/0x1f8
> > [   33.384168]  __driver_attach+0x78/0x3a0
> > [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> > [   33.392237]  driver_attach+0x4c/0x58
> > [   33.395910]  bus_add_driver+0x350/0x560
> > [   33.399854]  driver_register+0x23c/0x328
> > [   33.403886]  __platform_driver_register+0xd0/0xe0
> >
> > So, use dev_name(&pdev->dev), which fetches the platform device name from
> > the kobject(dev->kobj->name) of the device instead of the pdev->name.
> >
> > Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> > ---
> >  drivers/base/platform.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index dab0a5a..0e23aa2 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
> >         if (len != -ENODEV)
> >                 return len;
> >
> > -       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > +       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
> >
> >         return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> >  }
> > @@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> >                 return rc;
> >
> >         add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > -                       pdev->name);
> > +                       dev_name(&pdev->dev));
> 
> This is wrong fix and it causes ARM Vexpress board fail to boot under
> QEMU (but probably in real world as well). The problem is in not
> mached drivers. For example the pdev->name is "vexpress-syscfg" and
> dev_name(&pdev->dev) is "vexpress-syscfg.6.auto". The effect - none of
> AMBA devices are registered, including missing MMC device (mmci.c,
> arm,pl180).
> 
> One can see the error of missing root device:
> [   13.458982] VFS: Cannot open root device "mmcblk0" or
> unknown-block(0,0): error -6
> 
> ... also before there is a warning like:
> [    0.285029] ------------[ cut here ]------------
> [    0.285507] WARNING: CPU: 0 PID: 1 at
> /home/krzk/dev/yocto-proceq/build/workspace/sources/linux-mainline-next/drivers/bus/vexpress-config.c:183
> vexpress_config_init+0x90/0xe0
> [    0.285936] Modules linked in:
> [    0.286251] CPU: 0 PID: 1 Comm: swapper Tainted: G        W
> 5.1.0-rc6-next-20190429-g0593ae1f5df5 #27
> [    0.286507] Hardware name: ARM-Versatile Express
> [    0.286977] [<8010e05c>] (unwind_backtrace) from [<8010b76c>]
> (show_stack+0x10/0x14)
> [    0.287219] [<8010b76c>] (show_stack) from [<8011ac64>] (__warn+0xf8/0x110)
> [    0.287400] [<8011ac64>] (__warn) from [<8011ad94>]
> (warn_slowpath_null+0x40/0x48)
> [    0.287579] [<8011ad94>] (warn_slowpath_null) from [<809151bc>]
> (vexpress_config_init+0x90/0xe0)
> [    0.287811] [<809151bc>] (vexpress_config_init) from [<80102710>]
> (do_one_initcall+0x54/0x1b4)
> [    0.288014] [<80102710>] (do_one_initcall) from [<80900e84>]
> (kernel_init_freeable+0x12c/0x1c8)
> [    0.288214] [<80900e84>] (kernel_init_freeable) from [<80634048>]
> (kernel_init+0x8/0x110)
> [    0.288388] [<80634048>] (kernel_init) from [<801010e8>]
> (ret_from_fork+0x14/0x2c)
> [    0.288597] Exception stack(0x86835fb0 to 0x86835ff8)
> [    0.288882] 5fa0:                                     00000000
> 00000000 00000000 00000000
> [    0.289193] 5fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [    0.289479] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    0.289776] ---[ end trace 3f0995a2bae83983 ]---

Ick, that's not good, I've now reverted this patch from my tree, thanks
for letting me know.

greg k-h

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Greg KH]

On Mon, Apr 29, 2019 at 05:07:56PM +0200, Krzysztof Kozlowski wrote:
> On Tue, 23 Apr 2019 at 05:36, Venkata Narendra Kumar Gutta
> <vnkgutta@codeaurora.org> wrote:
> >
> > Platform core is using pdev->name as the platform device name to do
> > the binding of the devices with the drivers. But, when the platform
> > driver overrides the platform device name with dev_set_name(),
> > the pdev->name is pointing to a location which is freed and becomes
> 
> If pdev->name is invalid then it should be removed/fixed. Why leaving
> it pointing to wrong place and changing the users to something else?
> This looks like either duct-tape for real problem.
> 
> > an invalid parameter to do the binding match.
> >
> > use-after-free instance:
> >
> > [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> > [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> > [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
> >                         G S      W  O      4.19.30+ #3
> > [   33.346835] Hardware name: MTP (DT)
> > [   33.350419] Call trace:
> > [   33.352941]  dump_backtrace+0x0/0x3b8
> > [   33.356713]  show_stack+0x24/0x30
> > [   33.360119]  dump_stack+0x160/0x1d8
> > [   33.363709]  print_address_description+0x84/0x2e0
> > [   33.368549]  kasan_report+0x26c/0x2d0
> > [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> > [   33.377248]  strcmp+0x8c/0xb0
> > [   33.380306]  platform_match+0x70/0x1f8
> > [   33.384168]  __driver_attach+0x78/0x3a0
> > [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> > [   33.392237]  driver_attach+0x4c/0x58
> > [   33.395910]  bus_add_driver+0x350/0x560
> > [   33.399854]  driver_register+0x23c/0x328
> > [   33.403886]  __platform_driver_register+0xd0/0xe0
> >
> > So, use dev_name(&pdev->dev), which fetches the platform device name from
> > the kobject(dev->kobj->name) of the device instead of the pdev->name.
> >
> > Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> > ---
> >  drivers/base/platform.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index dab0a5a..0e23aa2 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
> >         if (len != -ENODEV)
> >                 return len;
> >
> > -       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > +       len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
> >
> >         return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> >  }
> > @@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> >                 return rc;
> >
> >         add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > -                       pdev->name);
> > +                       dev_name(&pdev->dev));
> 
> This is wrong fix and it causes ARM Vexpress board fail to boot under
> QEMU (but probably in real world as well). The problem is in not
> mached drivers. For example the pdev->name is "vexpress-syscfg" and
> dev_name(&pdev->dev) is "vexpress-syscfg.6.auto". The effect - none of
> AMBA devices are registered, including missing MMC device (mmci.c,
> arm,pl180).
> 
> One can see the error of missing root device:
> [   13.458982] VFS: Cannot open root device "mmcblk0" or
> unknown-block(0,0): error -6
> 
> ... also before there is a warning like:
> [    0.285029] ------------[ cut here ]------------
> [    0.285507] WARNING: CPU: 0 PID: 1 at
> /home/krzk/dev/yocto-proceq/build/workspace/sources/linux-mainline-next/drivers/bus/vexpress-config.c:183
> vexpress_config_init+0x90/0xe0
> [    0.285936] Modules linked in:
> [    0.286251] CPU: 0 PID: 1 Comm: swapper Tainted: G        W
> 5.1.0-rc6-next-20190429-g0593ae1f5df5 #27
> [    0.286507] Hardware name: ARM-Versatile Express
> [    0.286977] [<8010e05c>] (unwind_backtrace) from [<8010b76c>]
> (show_stack+0x10/0x14)
> [    0.287219] [<8010b76c>] (show_stack) from [<8011ac64>] (__warn+0xf8/0x110)
> [    0.287400] [<8011ac64>] (__warn) from [<8011ad94>]
> (warn_slowpath_null+0x40/0x48)
> [    0.287579] [<8011ad94>] (warn_slowpath_null) from [<809151bc>]
> (vexpress_config_init+0x90/0xe0)
> [    0.287811] [<809151bc>] (vexpress_config_init) from [<80102710>]
> (do_one_initcall+0x54/0x1b4)
> [    0.288014] [<80102710>] (do_one_initcall) from [<80900e84>]
> (kernel_init_freeable+0x12c/0x1c8)
> [    0.288214] [<80900e84>] (kernel_init_freeable) from [<80634048>]
> (kernel_init+0x8/0x110)
> [    0.288388] [<80634048>] (kernel_init) from [<801010e8>]
> (ret_from_fork+0x14/0x2c)
> [    0.288597] Exception stack(0x86835fb0 to 0x86835ff8)
> [    0.288882] 5fa0:                                     00000000
> 00000000 00000000 00000000
> [    0.289193] 5fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [    0.289479] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    0.289776] ---[ end trace 3f0995a2bae83983 ]---

Ick, that's not good, I've now reverted this patch from my tree, thanks
for letting me know.

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Guenter Roeck]

Hi,

On Mon, Apr 22, 2019 at 05:16:29PM -0700, Venkata Narendra Kumar Gutta wrote:
> Platform core is using pdev->name as the platform device name to do
> the binding of the devices with the drivers. But, when the platform
> driver overrides the platform device name with dev_set_name(),
> the pdev->name is pointing to a location which is freed and becomes
> an invalid parameter to do the binding match.
> 
> use-after-free instance:
> 
> [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
> 			G S      W  O      4.19.30+ #3
> [   33.346835] Hardware name: MTP (DT)
> [   33.350419] Call trace:
> [   33.352941]  dump_backtrace+0x0/0x3b8
> [   33.356713]  show_stack+0x24/0x30
> [   33.360119]  dump_stack+0x160/0x1d8
> [   33.363709]  print_address_description+0x84/0x2e0
> [   33.368549]  kasan_report+0x26c/0x2d0
> [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> [   33.377248]  strcmp+0x8c/0xb0
> [   33.380306]  platform_match+0x70/0x1f8
> [   33.384168]  __driver_attach+0x78/0x3a0
> [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> [   33.392237]  driver_attach+0x4c/0x58
> [   33.395910]  bus_add_driver+0x350/0x560
> [   33.399854]  driver_register+0x23c/0x328
> [   33.403886]  __platform_driver_register+0xd0/0xe0
> 
> So, use dev_name(&pdev->dev), which fetches the platform device name from
> the kobject(dev->kobj->name) of the device instead of the pdev->name.
> 
> Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>

This patch results in a large number of crashes (statistics: total: 349
pass: 244 fail: 105) in my boot tests (https://kerneltests.org/builders).
Affected architectures are (at least) arm, m68k, mips, ppc, and sh.
The reason for the crash is different for each architecture. Sometimes
the boot will stall, sometimes there is a crash, and sometimes the system
will fail to restart.

Here is an example for a log message, seen on arm (and m68k, but with ttyS
instead of ttySA).

WARNING: CPU: 0 PID: 1 at drivers/tty/tty_io.c:1349 tty_init_dev+0x14c/0x1a4
tty_init_dev: ttySA driver does not set tty->port. This will crash the kernel later. Fix the driver!

This is then indeed followed by a crash in tty_init_dev().

Bisect log for m68k attached below. Reverting this patch fixes the
problem at least for arm, m68k, and mips images.

Guenter

---
# bad: [3d17a1de96a233cf89bfbb5a77ebb1a05c420681] Add linux-next specific files for 20190429
# good: [085b7755808aa11f78ab9377257e1dad2e6fa4bb] Linux 5.1-rc6
git bisect start 'HEAD' 'v5.1-rc6'
# good: [48ea994d711ca2e66038741e549f3ebd3072e215] Merge remote-tracking branch 'crypto/master'
git bisect good 48ea994d711ca2e66038741e549f3ebd3072e215
# good: [2d49c5dbbd93045625927b6acf54bf43f86f97fd] Merge remote-tracking branch 'spi/for-next'
git bisect good 2d49c5dbbd93045625927b6acf54bf43f86f97fd
# bad: [7d38461c1c19569f7952c66913b38a78b2c51828] Merge remote-tracking branch 'staging/staging-next'
git bisect bad 7d38461c1c19569f7952c66913b38a78b2c51828
# bad: [b827800209cf30ed4e2d3a503044014b56f2b06f] Merge remote-tracking branch 'tty/tty-next'
git bisect bad b827800209cf30ed4e2d3a503044014b56f2b06f
# good: [e643fe145f03134a9de2b8996e11e03b8a0cd90a] Merge remote-tracking branch 'tip/auto-latest'
git bisect good e643fe145f03134a9de2b8996e11e03b8a0cd90a
# good: [cac573af020fbe8b16c1c769ed692126b8eceb69] Merge remote-tracking branch 'ipmi/for-next'
git bisect good cac573af020fbe8b16c1c769ed692126b8eceb69
# good: [ad74b8649beaf1a22cf8641324e3321fa0269d16] usb: typec: ucsi: Preliminary support for alternate modes
git bisect good ad74b8649beaf1a22cf8641324e3321fa0269d16
# bad: [9dc730c74af21b8403a9befba0f5f2e3bd9d6be4] Merge remote-tracking branch 'usb/usb-next'
git bisect bad 9dc730c74af21b8403a9befba0f5f2e3bd9d6be4
# good: [ab3a9f2ccc080d27873f76869c9a780be45e581e] acpi/hmat: fix an uninitialized memory_target
git bisect good ab3a9f2ccc080d27873f76869c9a780be45e581e
# good: [70283454c918f1d65de0ec50c45ef592d781bcae] livepatch: Replace klp_ktype_patch's default_attrs with groups
git bisect good 70283454c918f1d65de0ec50c45ef592d781bcae
# good: [33e39350ebd20fe6a77a51b8c21c3aa6b4a208cf] usb: xhci: add Immediate Data Transfer support
git bisect good 33e39350ebd20fe6a77a51b8c21c3aa6b4a208cf
# good: [5afa0a5ed3da85f64f27613a38daa1c4f69dd8ff] usb: xhci: add endpoint context tracing when an endpoint is added
git bisect good 5afa0a5ed3da85f64f27613a38daa1c4f69dd8ff
# bad: [a85b96e9e11d97a1fb4a683030d6aa98e1a872e8] Merge remote-tracking branch 'driver-core/driver-core-next'
git bisect bad a85b96e9e11d97a1fb4a683030d6aa98e1a872e8
# bad: [edb16da34b084c66763f29bee42b4e6bb33c3d66] driver core: platform: Fix the usage of platform device name(pdev->name)
git bisect bad edb16da34b084c66763f29bee42b4e6bb33c3d66
# first bad commit: [edb16da34b084c66763f29bee42b4e6bb33c3d66] driver core: platform: Fix the usage of platform device name(pdev->name)

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[Greg KH]

On Mon, Apr 29, 2019 at 11:20:58AM -0700, Guenter Roeck wrote:
> Hi,
> 
> On Mon, Apr 22, 2019 at 05:16:29PM -0700, Venkata Narendra Kumar Gutta wrote:
> > Platform core is using pdev->name as the platform device name to do
> > the binding of the devices with the drivers. But, when the platform
> > driver overrides the platform device name with dev_set_name(),
> > the pdev->name is pointing to a location which is freed and becomes
> > an invalid parameter to do the binding match.
> > 
> > use-after-free instance:
> > 
> > [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> > [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> > [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
> > 			G S      W  O      4.19.30+ #3
> > [   33.346835] Hardware name: MTP (DT)
> > [   33.350419] Call trace:
> > [   33.352941]  dump_backtrace+0x0/0x3b8
> > [   33.356713]  show_stack+0x24/0x30
> > [   33.360119]  dump_stack+0x160/0x1d8
> > [   33.363709]  print_address_description+0x84/0x2e0
> > [   33.368549]  kasan_report+0x26c/0x2d0
> > [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> > [   33.377248]  strcmp+0x8c/0xb0
> > [   33.380306]  platform_match+0x70/0x1f8
> > [   33.384168]  __driver_attach+0x78/0x3a0
> > [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> > [   33.392237]  driver_attach+0x4c/0x58
> > [   33.395910]  bus_add_driver+0x350/0x560
> > [   33.399854]  driver_register+0x23c/0x328
> > [   33.403886]  __platform_driver_register+0xd0/0xe0
> > 
> > So, use dev_name(&pdev->dev), which fetches the platform device name from
> > the kobject(dev->kobj->name) of the device instead of the pdev->name.
> > 
> > Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> 
> This patch results in a large number of crashes (statistics: total: 349
> pass: 244 fail: 105) in my boot tests (https://kerneltests.org/builders).
> Affected architectures are (at least) arm, m68k, mips, ppc, and sh.
> The reason for the crash is different for each architecture. Sometimes
> the boot will stall, sometimes there is a crash, and sometimes the system
> will fail to restart.
> 
> Here is an example for a log message, seen on arm (and m68k, but with ttyS
> instead of ttySA).
> 
> WARNING: CPU: 0 PID: 1 at drivers/tty/tty_io.c:1349 tty_init_dev+0x14c/0x1a4
> tty_init_dev: ttySA driver does not set tty->port. This will crash the kernel later. Fix the driver!
> 
> This is then indeed followed by a crash in tty_init_dev().
> 
> Bisect log for m68k attached below. Reverting this patch fixes the
> problem at least for arm, m68k, and mips images.
> 
> Guenter
> 
> ---
> # bad: [3d17a1de96a233cf89bfbb5a77ebb1a05c420681] Add linux-next specific files for 20190429
> # good: [085b7755808aa11f78ab9377257e1dad2e6fa4bb] Linux 5.1-rc6
> git bisect start 'HEAD' 'v5.1-rc6'
> # good: [48ea994d711ca2e66038741e549f3ebd3072e215] Merge remote-tracking branch 'crypto/master'
> git bisect good 48ea994d711ca2e66038741e549f3ebd3072e215
> # good: [2d49c5dbbd93045625927b6acf54bf43f86f97fd] Merge remote-tracking branch 'spi/for-next'
> git bisect good 2d49c5dbbd93045625927b6acf54bf43f86f97fd
> # bad: [7d38461c1c19569f7952c66913b38a78b2c51828] Merge remote-tracking branch 'staging/staging-next'
> git bisect bad 7d38461c1c19569f7952c66913b38a78b2c51828
> # bad: [b827800209cf30ed4e2d3a503044014b56f2b06f] Merge remote-tracking branch 'tty/tty-next'
> git bisect bad b827800209cf30ed4e2d3a503044014b56f2b06f
> # good: [e643fe145f03134a9de2b8996e11e03b8a0cd90a] Merge remote-tracking branch 'tip/auto-latest'
> git bisect good e643fe145f03134a9de2b8996e11e03b8a0cd90a
> # good: [cac573af020fbe8b16c1c769ed692126b8eceb69] Merge remote-tracking branch 'ipmi/for-next'
> git bisect good cac573af020fbe8b16c1c769ed692126b8eceb69
> # good: [ad74b8649beaf1a22cf8641324e3321fa0269d16] usb: typec: ucsi: Preliminary support for alternate modes
> git bisect good ad74b8649beaf1a22cf8641324e3321fa0269d16
> # bad: [9dc730c74af21b8403a9befba0f5f2e3bd9d6be4] Merge remote-tracking branch 'usb/usb-next'
> git bisect bad 9dc730c74af21b8403a9befba0f5f2e3bd9d6be4
> # good: [ab3a9f2ccc080d27873f76869c9a780be45e581e] acpi/hmat: fix an uninitialized memory_target
> git bisect good ab3a9f2ccc080d27873f76869c9a780be45e581e
> # good: [70283454c918f1d65de0ec50c45ef592d781bcae] livepatch: Replace klp_ktype_patch's default_attrs with groups
> git bisect good 70283454c918f1d65de0ec50c45ef592d781bcae
> # good: [33e39350ebd20fe6a77a51b8c21c3aa6b4a208cf] usb: xhci: add Immediate Data Transfer support
> git bisect good 33e39350ebd20fe6a77a51b8c21c3aa6b4a208cf
> # good: [5afa0a5ed3da85f64f27613a38daa1c4f69dd8ff] usb: xhci: add endpoint context tracing when an endpoint is added
> git bisect good 5afa0a5ed3da85f64f27613a38daa1c4f69dd8ff
> # bad: [a85b96e9e11d97a1fb4a683030d6aa98e1a872e8] Merge remote-tracking branch 'driver-core/driver-core-next'
> git bisect bad a85b96e9e11d97a1fb4a683030d6aa98e1a872e8
> # bad: [edb16da34b084c66763f29bee42b4e6bb33c3d66] driver core: platform: Fix the usage of platform device name(pdev->name)
> git bisect bad edb16da34b084c66763f29bee42b4e6bb33c3d66
> # first bad commit: [edb16da34b084c66763f29bee42b4e6bb33c3d66] driver core: platform: Fix the usage of platform device name(pdev->name)

Sorry, this was reverted from my tree about an hour before your message
as others also had problems.  The next linux next should have the revert
in it.

thanks,

greg k-h

Re: [PATCH] driver core: platform: Fix the usage of platform device name(pdev->name)

[sparkhuang]

On Mon, Apr 22, 2019 at 05:16:29PM -0700, Venkata Narendra Kumar Gutta wrote:
> Platform core is using pdev->name as the platform device name to do
> the binding of the devices with the drivers. But, when the platform
> driver overrides the platform device name with dev_set_name(),
> the pdev->name is pointing to a location which is freed and becomes
> an invalid parameter to do the binding match.
>
> use-after-free instance:
>
> [   33.325013] BUG: KASAN: use-after-free in strcmp+0x8c/0xb0
> [   33.330646] Read of size 1 at addr ffffffc10beae600 by task modprobe
> [   33.339068] CPU: 5 PID: 518 Comm: modprobe Tainted:
>                       G S      W  O      4.19.30+ #3
> [   33.346835] Hardware name: MTP (DT)
> [   33.350419] Call trace:
> [   33.352941]  dump_backtrace+0x0/0x3b8
> [   33.356713]  show_stack+0x24/0x30
> [   33.360119]  dump_stack+0x160/0x1d8
> [   33.363709]  print_address_description+0x84/0x2e0
> [   33.368549]  kasan_report+0x26c/0x2d0
> [   33.372322]  __asan_report_load1_noabort+0x2c/0x38
> [   33.377248]  strcmp+0x8c/0xb0
> [   33.380306]  platform_match+0x70/0x1f8
> [   33.384168]  __driver_attach+0x78/0x3a0
> [   33.388111]  bus_for_each_dev+0x13c/0x1b8
> [   33.392237]  driver_attach+0x4c/0x58
> [   33.395910]  bus_add_driver+0x350/0x560
> [   33.399854]  driver_register+0x23c/0x328
> [   33.403886]  __platform_driver_register+0xd0/0xe0

We also encountered the same problem, is there any solution?

> So, use dev_name(&pdev->dev), which fetches the platform device name from
> the kobject(dev->kobj->name) of the device instead of the pdev->name.
>
> Signed-off-by: Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
> ---
>  drivers/base/platform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dab0a5a..0e23aa2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -888,7 +888,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
>       if (len != -ENODEV)
>               return len;
>
> -     len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> +     len = snprintf(buf, PAGE_SIZE, "platform:%s\n", dev_name(&pdev->dev));
>
>       return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
>  }
> @@ -964,7 +964,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>               return rc;
>
>       add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> -                     pdev->name);
> +                     dev_name(&pdev->dev));
>       return 0;
>  }
>
> @@ -973,7 +973,7 @@ static const struct platform_device_id *platform_match_id(
>                       struct platform_device *pdev)
>  {
>       while (id->name[0]) {
> -             if (strcmp(pdev->name, id->name) == 0) {
> +             if (strcmp(dev_name(&pdev->dev), id->name) == 0) {
>                       pdev->id_entry = id;
>                       return id;
>               }
> @@ -1017,7 +1017,7 @@ static int platform_match(struct device *dev, struct device_driver *drv)
>               return platform_match_id(pdrv->id_table, pdev) != NULL;
>
>       /* fall-back to driver name match */
> -     return (strcmp(pdev->name, drv->name) == 0);
> +     return (strcmp(dev_name(&pdev->dev), drv->name) == 0);
>  }
>
>  #ifdef CONFIG_PM_SLEEP
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>