linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
@ 2013-11-30  4:55 Alexander Shiyan
  2013-11-30  9:34 ` Michael Grzeschik
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Shiyan @ 2013-11-30  4:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, Samuel Ortiz, Lee Jones, Shawn Guo,
	Sascha Hauer, Alexander Shiyan

If this driver being loaded from devicetree, the pdata is NULL.
This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
"fsl,mc13xxx-uses-touch" properties is specified.

mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
Unable to handle kernel NULL pointer dereference at virtual address 00000018
pgd = c0004000
[00000018] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT ARM
CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-next-20131105-00004-g201dd34-dirty #5
task: c7833bc0 ti: c7834000 task.ti: c7834000
PC is at mc13xxx_common_init+0x1fc/0x280
LR is at mfd_add_device+0x2a8/0x308
pc : [<c024875c>]    lr : [<c0248d74>]    psr: 20000013
sp : c7835d38  ip : 00000002  fp : c786a160
r10: 00000000  r9 : c786a170  r8 : 00000000
r7 : c06eab74  r6 : 00000000  r5 : 00000000  r4 : c79ffa10
r3 : 00000008  r2 : 20000013  r1 : c05079c8  r0 : c79ffa10
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: a0004000  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc78341c0)
Stack: (0xc7835d38 to 0xc7836000)
5d20:                                                       c0507958 c79ffa10
5d40: c791ab80 0000009b 00000000 c791ab80 c05de138 c023a998 c05a9854 c0293cf8
5d60: c0293ce0 c023a774 c023a998 00000000 c791ab80 c023a998 c79ffc00 c02393ac
5d80: c785a25c c7940e54 c791ab80 c791abb4 c791ab80 c023a4cc c791ab80 c05b2704
5da0: c791ab80 c02395e8 c791ab80 00000000 c791ab88 c0237938 00000000 c01d5ed8
5dc0: 60000013 c791ab80 c791ab80 c791ab80 00000000 c786a170 00000000 c06eaa04
5de0: c786a170 c0294d08 00000001 c79ffc00 c06eab74 c791ab80 00000000 c02953a8
5e00: c79f56a0 00000000 00000063 01312d00 00000000 c79ffc00 c79ffd78 c79ffc00
5e20: c06eaa04 00000000 c79ffd70 c0296cbc c780a840 c02978c0 00000000 c051054c
5e40: c79ffd70 c787c1f0 00000000 c0462b1c 00000000 00000002 c787c1f0 c786a170
5e60: c05b2864 c786a1a4 c05b2864 00000000 c05c5800 c0579430 00000000 c023b614
5e80: c023b5fc c786a170 c05de138 c023a774 00000000 c786a170 c05b2864 c786a1a4
5ea0: 00000000 c023a994 00000000 c05b2864 c023a908 c0239034 c7825d6c c785e610
5ec0: c05b2864 c7a0bae0 c05a9138 c02397dc c051054c c01bd2f8 c05b2864 c05b2864
5ee0: c0585094 579da752 00000000 c023ad38 00000000 00000006 c0585094 c0564a24
5f00: 0000047a 00000000 579da752 00000000 00000000 a0000053 c059b070 00000001
5f20: c06eefc9 c044c6a0 000000a7 c002cd48 c7835f5c c0036f44 c051aae4 c053d7f4
5f40: 00000006 00000006 c059b064 00000006 00000006 c0585094 c058db68 c0564480
5f60: c05850a0 c05c5800 000000a7 c0564c18 00000006 00000006 c0564480 c7833bc0
5f80: c7835f9c c0036da4 00000000 c043013c 00000000 00000000 00000000 00000000
5fa0: 00000000 c0430144 00000000 c0009490 00000000 00000000 00000000 00000000
5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 df7f9b78 3b477df6
[<c024875c>] (mc13xxx_common_init+0x1fc/0x280) from [<c0293cf8>] (spi_drv_probe+0x18/0x1c)
[<c0293cf8>] (spi_drv_probe+0x18/0x1c) from [<c023a774>] (driver_probe_device+0x78/0x20c)
[<c023a774>] (driver_probe_device+0x78/0x20c) from [<c02393ac>] (bus_for_each_drv+0x48/0x90)
[<c02393ac>] (bus_for_each_drv+0x48/0x90) from [<c023a4cc>] (device_attach+0x74/0x80)
[<c023a4cc>] (device_attach+0x74/0x80) from [<c02395e8>] (bus_probe_device+0x88/0xb0)
[<c02395e8>] (bus_probe_device+0x88/0xb0) from [<c0237938>] (device_add+0x31c/0x4c4)
[<c0237938>] (device_add+0x31c/0x4c4) from [<c0294d08>] (spi_add_device+0xa4/0x118)
[<c0294d08>] (spi_add_device+0xa4/0x118) from [<c02953a8>] (spi_register_master+0x53c/0x6a0)
[<c02953a8>] (spi_register_master+0x53c/0x6a0) from [<c0296cbc>] (spi_bitbang_start+0x88/0x100)
[<c0296cbc>] (spi_bitbang_start+0x88/0x100) from [<c02978c0>] (spi_imx_probe+0x3a8/0x478)
[<c02978c0>] (spi_imx_probe+0x3a8/0x478) from [<c023b614>] (platform_drv_probe+0x18/0x48)
[<c023b614>] (platform_drv_probe+0x18/0x48) from [<c023a774>] (driver_probe_device+0x78/0x20c)
[<c023a774>] (driver_probe_device+0x78/0x20c) from [<c023a994>] (__driver_attach+0x8c/0x90)
[<c023a994>] (__driver_attach+0x8c/0x90) from [<c0239034>] (bus_for_each_dev+0x5c/0x8c)
[<c0239034>] (bus_for_each_dev+0x5c/0x8c) from [<c02397dc>] (bus_add_driver+0xd4/0x1c8)
[<c02397dc>] (bus_add_driver+0xd4/0x1c8) from [<c023ad38>] (driver_register+0x78/0xf4)
[<c023ad38>] (driver_register+0x78/0xf4) from [<c0564a24>] (do_one_initcall+0x5c/0x168)
[<c0564a24>] (do_one_initcall+0x5c/0x168) from [<c0564c18>] (kernel_init_freeable+0xe8/0x1a8)
[<c0564c18>] (kernel_init_freeable+0xe8/0x1a8) from [<c0430144>] (kernel_init+0x8/0x110)
[<c0430144>] (kernel_init+0x8/0x110) from [<c0009490>] (ret_from_fork+0x14/0x24)
Code: eaffffdb e3a03008 e1a00004 e59f107c (e5962018)
---[ end trace 71265dd116f3967e ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/mfd/mc13xxx-core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index dbbf8ee..625633a 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -682,14 +682,14 @@ err_revision:
 	if (mc13xxx->flags & MC13XXX_USE_ADC)
 		mc13xxx_add_subdevice(mc13xxx, "%s-adc");
 
-	if (mc13xxx->flags & MC13XXX_USE_CODEC)
+	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec",
 					pdata->codec, sizeof(*pdata->codec));
 
 	if (mc13xxx->flags & MC13XXX_USE_RTC)
 		mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
 
-	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
 				&pdata->touch, sizeof(pdata->touch));
 
@@ -701,6 +701,10 @@ err_revision:
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
 				pdata->buttons, sizeof(*pdata->buttons));
 	} else {
+		if (mc13xxx->flags & MC13XXX_USE_CODEC)
+			mc13xxx_add_subdevice(mc13xxx, "%s-codec");
+		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+			mc13xxx_add_subdevice(mc13xxx, "%s-ts");
 		mc13xxx_add_subdevice(mc13xxx, "%s-regulator");
 		mc13xxx_add_subdevice(mc13xxx, "%s-led");
 		mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton");
-- 
1.8.3.2


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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-11-30  4:55 [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init Alexander Shiyan
@ 2013-11-30  9:34 ` Michael Grzeschik
  2013-11-30  9:52   ` Alexander Shiyan
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Grzeschik @ 2013-11-30  9:34 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-kernel, linux-arm-kernel, Samuel Ortiz, Lee Jones,
	Shawn Guo, Sascha Hauer

On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> If this driver being loaded from devicetree, the pdata is NULL.
> This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> "fsl,mc13xxx-uses-touch" properties is specified.
> 
> mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> Unable to handle kernel NULL pointer dereference at virtual address 00000018
> pgd = c0004000
> [00000018] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT ARM
> CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-next-20131105-00004-g201dd34-dirty #5
> task: c7833bc0 ti: c7834000 task.ti: c7834000
> PC is at mc13xxx_common_init+0x1fc/0x280
> LR is at mfd_add_device+0x2a8/0x308
> pc : [<c024875c>]    lr : [<c0248d74>]    psr: 20000013
> sp : c7835d38  ip : 00000002  fp : c786a160
> r10: 00000000  r9 : c786a170  r8 : 00000000
> r7 : c06eab74  r6 : 00000000  r5 : 00000000  r4 : c79ffa10
> r3 : 00000008  r2 : 20000013  r1 : c05079c8  r0 : c79ffa10
> Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 00000017
> Process swapper (pid: 1, stack limit = 0xc78341c0)
> Stack: (0xc7835d38 to 0xc7836000)
> 5d20:                                                       c0507958 c79ffa10
> 5d40: c791ab80 0000009b 00000000 c791ab80 c05de138 c023a998 c05a9854 c0293cf8
> 5d60: c0293ce0 c023a774 c023a998 00000000 c791ab80 c023a998 c79ffc00 c02393ac
> 5d80: c785a25c c7940e54 c791ab80 c791abb4 c791ab80 c023a4cc c791ab80 c05b2704
> 5da0: c791ab80 c02395e8 c791ab80 00000000 c791ab88 c0237938 00000000 c01d5ed8
> 5dc0: 60000013 c791ab80 c791ab80 c791ab80 00000000 c786a170 00000000 c06eaa04
> 5de0: c786a170 c0294d08 00000001 c79ffc00 c06eab74 c791ab80 00000000 c02953a8
> 5e00: c79f56a0 00000000 00000063 01312d00 00000000 c79ffc00 c79ffd78 c79ffc00
> 5e20: c06eaa04 00000000 c79ffd70 c0296cbc c780a840 c02978c0 00000000 c051054c
> 5e40: c79ffd70 c787c1f0 00000000 c0462b1c 00000000 00000002 c787c1f0 c786a170
> 5e60: c05b2864 c786a1a4 c05b2864 00000000 c05c5800 c0579430 00000000 c023b614
> 5e80: c023b5fc c786a170 c05de138 c023a774 00000000 c786a170 c05b2864 c786a1a4
> 5ea0: 00000000 c023a994 00000000 c05b2864 c023a908 c0239034 c7825d6c c785e610
> 5ec0: c05b2864 c7a0bae0 c05a9138 c02397dc c051054c c01bd2f8 c05b2864 c05b2864
> 5ee0: c0585094 579da752 00000000 c023ad38 00000000 00000006 c0585094 c0564a24
> 5f00: 0000047a 00000000 579da752 00000000 00000000 a0000053 c059b070 00000001
> 5f20: c06eefc9 c044c6a0 000000a7 c002cd48 c7835f5c c0036f44 c051aae4 c053d7f4
> 5f40: 00000006 00000006 c059b064 00000006 00000006 c0585094 c058db68 c0564480
> 5f60: c05850a0 c05c5800 000000a7 c0564c18 00000006 00000006 c0564480 c7833bc0
> 5f80: c7835f9c c0036da4 00000000 c043013c 00000000 00000000 00000000 00000000
> 5fa0: 00000000 c0430144 00000000 c0009490 00000000 00000000 00000000 00000000
> 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 df7f9b78 3b477df6
> [<c024875c>] (mc13xxx_common_init+0x1fc/0x280) from [<c0293cf8>] (spi_drv_probe+0x18/0x1c)
> [<c0293cf8>] (spi_drv_probe+0x18/0x1c) from [<c023a774>] (driver_probe_device+0x78/0x20c)
> [<c023a774>] (driver_probe_device+0x78/0x20c) from [<c02393ac>] (bus_for_each_drv+0x48/0x90)
> [<c02393ac>] (bus_for_each_drv+0x48/0x90) from [<c023a4cc>] (device_attach+0x74/0x80)
> [<c023a4cc>] (device_attach+0x74/0x80) from [<c02395e8>] (bus_probe_device+0x88/0xb0)
> [<c02395e8>] (bus_probe_device+0x88/0xb0) from [<c0237938>] (device_add+0x31c/0x4c4)
> [<c0237938>] (device_add+0x31c/0x4c4) from [<c0294d08>] (spi_add_device+0xa4/0x118)
> [<c0294d08>] (spi_add_device+0xa4/0x118) from [<c02953a8>] (spi_register_master+0x53c/0x6a0)
> [<c02953a8>] (spi_register_master+0x53c/0x6a0) from [<c0296cbc>] (spi_bitbang_start+0x88/0x100)
> [<c0296cbc>] (spi_bitbang_start+0x88/0x100) from [<c02978c0>] (spi_imx_probe+0x3a8/0x478)
> [<c02978c0>] (spi_imx_probe+0x3a8/0x478) from [<c023b614>] (platform_drv_probe+0x18/0x48)
> [<c023b614>] (platform_drv_probe+0x18/0x48) from [<c023a774>] (driver_probe_device+0x78/0x20c)
> [<c023a774>] (driver_probe_device+0x78/0x20c) from [<c023a994>] (__driver_attach+0x8c/0x90)
> [<c023a994>] (__driver_attach+0x8c/0x90) from [<c0239034>] (bus_for_each_dev+0x5c/0x8c)
> [<c0239034>] (bus_for_each_dev+0x5c/0x8c) from [<c02397dc>] (bus_add_driver+0xd4/0x1c8)
> [<c02397dc>] (bus_add_driver+0xd4/0x1c8) from [<c023ad38>] (driver_register+0x78/0xf4)
> [<c023ad38>] (driver_register+0x78/0xf4) from [<c0564a24>] (do_one_initcall+0x5c/0x168)
> [<c0564a24>] (do_one_initcall+0x5c/0x168) from [<c0564c18>] (kernel_init_freeable+0xe8/0x1a8)
> [<c0564c18>] (kernel_init_freeable+0xe8/0x1a8) from [<c0430144>] (kernel_init+0x8/0x110)
> [<c0430144>] (kernel_init+0x8/0x110) from [<c0009490>] (ret_from_fork+0x14/0x24)
> Code: eaffffdb e3a03008 e1a00004 e59f107c (e5962018)
> ---[ end trace 71265dd116f3967e ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/mfd/mc13xxx-core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> index dbbf8ee..625633a 100644
> --- a/drivers/mfd/mc13xxx-core.c
> +++ b/drivers/mfd/mc13xxx-core.c
> @@ -682,14 +682,14 @@ err_revision:
>  	if (mc13xxx->flags & MC13XXX_USE_ADC)
>  		mc13xxx_add_subdevice(mc13xxx, "%s-adc");
>  
> -	if (mc13xxx->flags & MC13XXX_USE_CODEC)
> +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec",
>  					pdata->codec, sizeof(*pdata->codec));
>  
>  	if (mc13xxx->flags & MC13XXX_USE_RTC)
>  		mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
>  
> -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)

Why do we check for CODEC if the Touchscreen should be used?

>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
>  				&pdata->touch, sizeof(pdata->touch));
>  
> @@ -701,6 +701,10 @@ err_revision:
>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
>  				pdata->buttons, sizeof(*pdata->buttons));
>  	} else {
> +		if (mc13xxx->flags & MC13XXX_USE_CODEC)
> +			mc13xxx_add_subdevice(mc13xxx, "%s-codec");
> +		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> +			mc13xxx_add_subdevice(mc13xxx, "%s-ts");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-regulator");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-led");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton");
> -- 
> 1.8.3.2
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-11-30  9:34 ` Michael Grzeschik
@ 2013-11-30  9:52   ` Alexander Shiyan
  2013-12-02 10:56     ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Shiyan @ 2013-11-30  9:52 UTC (permalink / raw)
  To: Michael Grzeschik
  Cc: Samuel Ortiz, linux-kernel, Sascha Hauer, Shawn Guo, Lee Jones,
	linux-arm-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 815 bytes --]

> On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> > If this driver being loaded from devicetree, the pdata is NULL.
> > This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> > "fsl,mc13xxx-uses-touch" properties is specified.
> > 
> > mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> > Unable to handle kernel NULL pointer dereference at virtual address 00000018
...
> > -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> > +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
> 
> Why do we check for CODEC if the Touchscreen should be used?

Oops, my fault, copy/paste bug.
I'll send the corrected version.

---
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-11-30  9:52   ` Alexander Shiyan
@ 2013-12-02 10:56     ` Lee Jones
  2013-12-02 11:11       ` Alexander Shiyan
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2013-12-02 10:56 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: Michael Grzeschik, Samuel Ortiz, linux-kernel, Sascha Hauer,
	Shawn Guo, linux-arm-kernel

On Sat, 30 Nov 2013, Alexander Shiyan wrote:

> > On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> > > If this driver being loaded from devicetree, the pdata is NULL.
> > > This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> > > "fsl,mc13xxx-uses-touch" properties is specified.
> > > 
> > > mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> > > Unable to handle kernel NULL pointer dereference at virtual address 00000018
> ...
> > > -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> > > +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
> > 
> > Why do we check for CODEC if the Touchscreen should be used?
> 
> Oops, my fault, copy/paste bug.
> I'll send the corrected version.

No, please don't.

Just provide proper Device Tree support.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-12-02 10:56     ` Lee Jones
@ 2013-12-02 11:11       ` Alexander Shiyan
  2013-12-02 11:17         ` Lee Jones
  2013-12-02 11:53         ` Michael Grzeschik
  0 siblings, 2 replies; 10+ messages in thread
From: Alexander Shiyan @ 2013-12-02 11:11 UTC (permalink / raw)
  To: Lee Jones
  Cc: Samuel Ortiz, linux-kernel, Michael Grzeschik, Sascha Hauer,
	Shawn Guo, linux-arm-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1362 bytes --]




Понедельник,  2 декабря 2013, 10:56 UTC от Lee Jones <lee.jones@linaro.org>:
> On Sat, 30 Nov 2013, Alexander Shiyan wrote:
> 
> > > On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> > > > If this driver being loaded from devicetree, the pdata is NULL.
> > > > This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> > > > "fsl,mc13xxx-uses-touch" properties is specified.
> > > > 
> > > > mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> > > > Unable to handle kernel NULL pointer dereference at virtual address 00000018
> > ...
> > > > -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> > > > +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
> > > 
> > > Why do we check for CODEC if the Touchscreen should be used?
> > 
> > Oops, my fault, copy/paste bug.
> > I'll send the corrected version.
> 
> No, please don't.
> 
> Just provide proper Device Tree support.

I still have two incomplete MC13XXX DT-related patches.
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175099.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/182116.html

I'll resume to work on this (and CODEC & TS too).
Thanks.

---
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-12-02 11:11       ` Alexander Shiyan
@ 2013-12-02 11:17         ` Lee Jones
  2013-12-02 11:53         ` Michael Grzeschik
  1 sibling, 0 replies; 10+ messages in thread
From: Lee Jones @ 2013-12-02 11:17 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: Samuel Ortiz, linux-kernel, Michael Grzeschik, Sascha Hauer,
	Shawn Guo, linux-arm-kernel

On Mon, 02 Dec 2013, Alexander Shiyan wrote:

> 
> 
> 
> Понедельник,  2 декабря 2013, 10:56 UTC от Lee Jones <lee.jones@linaro.org>:
> > On Sat, 30 Nov 2013, Alexander Shiyan wrote:
> > 
> > > > On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> > > > > If this driver being loaded from devicetree, the pdata is NULL.
> > > > > This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> > > > > "fsl,mc13xxx-uses-touch" properties is specified.
> > > > > 
> > > > > mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> > > > > Unable to handle kernel NULL pointer dereference at virtual address 00000018
> > > ...
> > > > > -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> > > > > +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
> > > > 
> > > > Why do we check for CODEC if the Touchscreen should be used?
> > > 
> > > Oops, my fault, copy/paste bug.
> > > I'll send the corrected version.
> > 
> > No, please don't.
> > 
> > Just provide proper Device Tree support.
> 
> I still have two incomplete MC13XXX DT-related patches.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175099.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/182116.html

Patches from June and July are no longer pending. They need to be
refreshed if you stand any chance of then being accepted.

> I'll resume to work on this (and CODEC & TS too).

Okay, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-12-02 11:11       ` Alexander Shiyan
  2013-12-02 11:17         ` Lee Jones
@ 2013-12-02 11:53         ` Michael Grzeschik
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Grzeschik @ 2013-12-02 11:53 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: Lee Jones, Samuel Ortiz, linux-kernel, Sascha Hauer, Shawn Guo,
	linux-arm-kernel

On Mon, Dec 02, 2013 at 03:11:03PM +0400, Alexander Shiyan wrote:
> 
> 
> 
> Понедельник,  2 декабря 2013, 10:56 UTC от Lee Jones <lee.jones@linaro.org>:
> > On Sat, 30 Nov 2013, Alexander Shiyan wrote:
> > 
> > > > On Sat, Nov 30, 2013 at 08:55:37AM +0400, Alexander Shiyan wrote:
> > > > > If this driver being loaded from devicetree, the pdata is NULL.
> > > > > This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> > > > > "fsl,mc13xxx-uses-touch" properties is specified.
> > > > > 
> > > > > mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> > > > > Unable to handle kernel NULL pointer dereference at virtual address 00000018
> > > ...
> > > > > -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> > > > > +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
> > > > 
> > > > Why do we check for CODEC if the Touchscreen should be used?
> > > 
> > > Oops, my fault, copy/paste bug.
> > > I'll send the corrected version.
> > 
> > No, please don't.
> > 
> > Just provide proper Device Tree support.
> 
> I still have two incomplete MC13XXX DT-related patches.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/175099.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/182116.html
> 
> I'll resume to work on this (and CODEC & TS too).

When you keep working on it, you can also look into that tree:

http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/v3.11/topic/mc13xxx

I currently have no time and no capable hardware on my desk to keep
working on this. But it would be great if this code will not keep
rotting any further.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-11-30  9:53 Alexander Shiyan
  2013-12-02  8:30 ` Philippe Rétornaz
@ 2013-12-02 10:54 ` Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Lee Jones @ 2013-12-02 10:54 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-kernel, linux-arm-kernel, Samuel Ortiz, Shawn Guo, Sascha Hauer

On Sat, 30 Nov 2013, Alexander Shiyan wrote:

> If this driver being loaded from devicetree, the pdata is NULL.
> This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> "fsl,mc13xxx-uses-touch" properties is specified.

<snip>

The original code is pretty gross. This stuff shouldn't be identifying
_support_ in Device Tree, then dragging in configuration from platform
code.

Rather than patch it up with little plasters (band aids), can we fix
it properly please?

> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/mfd/mc13xxx-core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> index dbbf8ee..625633a 100644
> --- a/drivers/mfd/mc13xxx-core.c
> +++ b/drivers/mfd/mc13xxx-core.c
> @@ -682,14 +682,14 @@ err_revision:
>  	if (mc13xxx->flags & MC13XXX_USE_ADC)
>  		mc13xxx_add_subdevice(mc13xxx, "%s-adc");
>  
> -	if (mc13xxx->flags & MC13XXX_USE_CODEC)
> +	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec",
>  					pdata->codec, sizeof(*pdata->codec));
>  
>  	if (mc13xxx->flags & MC13XXX_USE_RTC)
>  		mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
>  
> -	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> +	if ((mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN) && pdata)
>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
>  				&pdata->touch, sizeof(pdata->touch));
>  
> @@ -701,6 +701,10 @@ err_revision:
>  		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
>  				pdata->buttons, sizeof(*pdata->buttons));
>  	} else {
> +		if (mc13xxx->flags & MC13XXX_USE_CODEC)
> +			mc13xxx_add_subdevice(mc13xxx, "%s-codec");
> +		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
> +			mc13xxx_add_subdevice(mc13xxx, "%s-ts");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-regulator");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-led");
>  		mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton");

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
  2013-11-30  9:53 Alexander Shiyan
@ 2013-12-02  8:30 ` Philippe Rétornaz
  2013-12-02 10:54 ` Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Philippe Rétornaz @ 2013-12-02  8:30 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-kernel, Samuel Ortiz, Sascha Hauer, Shawn Guo, Lee Jones,
	linux-arm-kernel

Le 30/11/2013 10:53, Alexander Shiyan a écrit :
> If this driver being loaded from devicetree, the pdata is NULL. This
> cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
> "fsl,mc13xxx-uses-touch" properties is specified.
>

Well, the codec and touch are not supported if DT is used, thus why
bothering ? Just don't specify thoses properties.

IMHO it would be better to either implement full DT support in the
subdevices or removing the check for the fsl,mc13xxx-uses-codec and
fsl,mc13xxx-uses-touch in mc13xxx_probe_flags_dt().
If it's not supported let's not lie about it.


Regards,

Philippe

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

* [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init
@ 2013-11-30  9:53 Alexander Shiyan
  2013-12-02  8:30 ` Philippe Rétornaz
  2013-12-02 10:54 ` Lee Jones
  0 siblings, 2 replies; 10+ messages in thread
From: Alexander Shiyan @ 2013-11-30  9:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, Samuel Ortiz, Lee Jones, Shawn Guo,
	Sascha Hauer, Alexander Shiyan

If this driver being loaded from devicetree, the pdata is NULL.
This cause kernel Oops when "fsl,mc13xxx-uses-codec" and/or
"fsl,mc13xxx-uses-touch" properties is specified.

mc13xxx spi0.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
Unable to handle kernel NULL pointer dereference at virtual address 00000018
pgd = c0004000
[00000018] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT ARM
CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-next-20131105-00004-g201dd34-dirty #5
task: c7833bc0 ti: c7834000 task.ti: c7834000
PC is at mc13xxx_common_init+0x1fc/0x280
LR is at mfd_add_device+0x2a8/0x308
pc : [<c024875c>]    lr : [<c0248d74>]    psr: 20000013
sp : c7835d38  ip : 00000002  fp : c786a160
r10: 00000000  r9 : c786a170  r8 : 00000000
r7 : c06eab74  r6 : 00000000  r5 : 00000000  r4 : c79ffa10
r3 : 00000008  r2 : 20000013  r1 : c05079c8  r0 : c79ffa10
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: a0004000  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc78341c0)
Stack: (0xc7835d38 to 0xc7836000)
5d20:                                                       c0507958 c79ffa10
5d40: c791ab80 0000009b 00000000 c791ab80 c05de138 c023a998 c05a9854 c0293cf8
5d60: c0293ce0 c023a774 c023a998 00000000 c791ab80 c023a998 c79ffc00 c02393ac
5d80: c785a25c c7940e54 c791ab80 c791abb4 c791ab80 c023a4cc c791ab80 c05b2704
5da0: c791ab80 c02395e8 c791ab80 00000000 c791ab88 c0237938 00000000 c01d5ed8
5dc0: 60000013 c791ab80 c791ab80 c791ab80 00000000 c786a170 00000000 c06eaa04
5de0: c786a170 c0294d08 00000001 c79ffc00 c06eab74 c791ab80 00000000 c02953a8
5e00: c79f56a0 00000000 00000063 01312d00 00000000 c79ffc00 c79ffd78 c79ffc00
5e20: c06eaa04 00000000 c79ffd70 c0296cbc c780a840 c02978c0 00000000 c051054c
5e40: c79ffd70 c787c1f0 00000000 c0462b1c 00000000 00000002 c787c1f0 c786a170
5e60: c05b2864 c786a1a4 c05b2864 00000000 c05c5800 c0579430 00000000 c023b614
5e80: c023b5fc c786a170 c05de138 c023a774 00000000 c786a170 c05b2864 c786a1a4
5ea0: 00000000 c023a994 00000000 c05b2864 c023a908 c0239034 c7825d6c c785e610
5ec0: c05b2864 c7a0bae0 c05a9138 c02397dc c051054c c01bd2f8 c05b2864 c05b2864
5ee0: c0585094 579da752 00000000 c023ad38 00000000 00000006 c0585094 c0564a24
5f00: 0000047a 00000000 579da752 00000000 00000000 a0000053 c059b070 00000001
5f20: c06eefc9 c044c6a0 000000a7 c002cd48 c7835f5c c0036f44 c051aae4 c053d7f4
5f40: 00000006 00000006 c059b064 00000006 00000006 c0585094 c058db68 c0564480
5f60: c05850a0 c05c5800 000000a7 c0564c18 00000006 00000006 c0564480 c7833bc0
5f80: c7835f9c c0036da4 00000000 c043013c 00000000 00000000 00000000 00000000
5fa0: 00000000 c0430144 00000000 c0009490 00000000 00000000 00000000 00000000
5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 df7f9b78 3b477df6
[<c024875c>] (mc13xxx_common_init+0x1fc/0x280) from [<c0293cf8>] (spi_drv_probe+0x18/0x1c)
[<c0293cf8>] (spi_drv_probe+0x18/0x1c) from [<c023a774>] (driver_probe_device+0x78/0x20c)
[<c023a774>] (driver_probe_device+0x78/0x20c) from [<c02393ac>] (bus_for_each_drv+0x48/0x90)
[<c02393ac>] (bus_for_each_drv+0x48/0x90) from [<c023a4cc>] (device_attach+0x74/0x80)
[<c023a4cc>] (device_attach+0x74/0x80) from [<c02395e8>] (bus_probe_device+0x88/0xb0)
[<c02395e8>] (bus_probe_device+0x88/0xb0) from [<c0237938>] (device_add+0x31c/0x4c4)
[<c0237938>] (device_add+0x31c/0x4c4) from [<c0294d08>] (spi_add_device+0xa4/0x118)
[<c0294d08>] (spi_add_device+0xa4/0x118) from [<c02953a8>] (spi_register_master+0x53c/0x6a0)
[<c02953a8>] (spi_register_master+0x53c/0x6a0) from [<c0296cbc>] (spi_bitbang_start+0x88/0x100)
[<c0296cbc>] (spi_bitbang_start+0x88/0x100) from [<c02978c0>] (spi_imx_probe+0x3a8/0x478)
[<c02978c0>] (spi_imx_probe+0x3a8/0x478) from [<c023b614>] (platform_drv_probe+0x18/0x48)
[<c023b614>] (platform_drv_probe+0x18/0x48) from [<c023a774>] (driver_probe_device+0x78/0x20c)
[<c023a774>] (driver_probe_device+0x78/0x20c) from [<c023a994>] (__driver_attach+0x8c/0x90)
[<c023a994>] (__driver_attach+0x8c/0x90) from [<c0239034>] (bus_for_each_dev+0x5c/0x8c)
[<c0239034>] (bus_for_each_dev+0x5c/0x8c) from [<c02397dc>] (bus_add_driver+0xd4/0x1c8)
[<c02397dc>] (bus_add_driver+0xd4/0x1c8) from [<c023ad38>] (driver_register+0x78/0xf4)
[<c023ad38>] (driver_register+0x78/0xf4) from [<c0564a24>] (do_one_initcall+0x5c/0x168)
[<c0564a24>] (do_one_initcall+0x5c/0x168) from [<c0564c18>] (kernel_init_freeable+0xe8/0x1a8)
[<c0564c18>] (kernel_init_freeable+0xe8/0x1a8) from [<c0430144>] (kernel_init+0x8/0x110)
[<c0430144>] (kernel_init+0x8/0x110) from [<c0009490>] (ret_from_fork+0x14/0x24)
Code: eaffffdb e3a03008 e1a00004 e59f107c (e5962018)
---[ end trace 71265dd116f3967e ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/mfd/mc13xxx-core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index dbbf8ee..625633a 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -682,14 +682,14 @@ err_revision:
 	if (mc13xxx->flags & MC13XXX_USE_ADC)
 		mc13xxx_add_subdevice(mc13xxx, "%s-adc");
 
-	if (mc13xxx->flags & MC13XXX_USE_CODEC)
+	if ((mc13xxx->flags & MC13XXX_USE_CODEC) && pdata)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec",
 					pdata->codec, sizeof(*pdata->codec));
 
 	if (mc13xxx->flags & MC13XXX_USE_RTC)
 		mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
 
-	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+	if ((mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN) && pdata)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
 				&pdata->touch, sizeof(pdata->touch));
 
@@ -701,6 +701,10 @@ err_revision:
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
 				pdata->buttons, sizeof(*pdata->buttons));
 	} else {
+		if (mc13xxx->flags & MC13XXX_USE_CODEC)
+			mc13xxx_add_subdevice(mc13xxx, "%s-codec");
+		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+			mc13xxx_add_subdevice(mc13xxx, "%s-ts");
 		mc13xxx_add_subdevice(mc13xxx, "%s-regulator");
 		mc13xxx_add_subdevice(mc13xxx, "%s-led");
 		mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton");
-- 
1.8.3.2


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

end of thread, other threads:[~2013-12-02 11:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-30  4:55 [PATCH RESEND] mfd: mc13xxx: Fix null pointer dereference in mc13xxx_common_init Alexander Shiyan
2013-11-30  9:34 ` Michael Grzeschik
2013-11-30  9:52   ` Alexander Shiyan
2013-12-02 10:56     ` Lee Jones
2013-12-02 11:11       ` Alexander Shiyan
2013-12-02 11:17         ` Lee Jones
2013-12-02 11:53         ` Michael Grzeschik
2013-11-30  9:53 Alexander Shiyan
2013-12-02  8:30 ` Philippe Rétornaz
2013-12-02 10:54 ` Lee Jones

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).