[PATCH v1 1/1] drivers: base: Fix NULL pointer exception in __platform_driver_probe()

[PATCH v1 1/1] drivers: base: Fix NULL pointer exception in __platform_driver_probe()

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

If platform bus driver registration is failed then, accessing
platform bus spin lock (&drv->driver.bus->p->klist_drivers.k_lock)
in __platform_driver_probe() without verifying the return value
__platform_driver_register() can lead to NULL pointer exception.

So check the return value before attempting the spin lock.

One such example is below:

For a custom usecase, I have intentionally failed the platform bus
registration and I expected all the platform device/driver
registrations to fail gracefully. But I came across this panic
issue.

[    1.331067] BUG: kernel NULL pointer dereference, address: 00000000000000c8
[    1.331118] #PF: supervisor write access in kernel mode
[    1.331163] #PF: error_code(0x0002) - not-present page
[    1.331208] PGD 0 P4D 0
[    1.331233] Oops: 0002 [#1] PREEMPT SMP
[    1.331268] CPU: 3 PID: 1 Comm: swapper/0 Tainted: G        W         5.6.0-00049-g670d35fb0144 #165
[    1.331341] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[    1.331406] RIP: 0010:_raw_spin_lock+0x15/0x30
[    1.331588] RSP: 0000:ffffc9000001be70 EFLAGS: 00010246
[    1.331632] RAX: 0000000000000000 RBX: 00000000000000c8 RCX: 0000000000000001
[    1.331696] RDX: 0000000000000001 RSI: 0000000000000092 RDI: 0000000000000000
[    1.331754] RBP: 00000000ffffffed R08: 0000000000000501 R09: 0000000000000001
[    1.331817] R10: ffff88817abcc520 R11: 0000000000000670 R12: 00000000ffffffed
[    1.331881] R13: ffffffff82dbc268 R14: ffffffff832f070a R15: 0000000000000000
[    1.331945] FS:  0000000000000000(0000) GS:ffff88817bd80000(0000) knlGS:0000000000000000
[    1.332008] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.332062] CR2: 00000000000000c8 CR3: 000000000681e001 CR4: 00000000003606e0
[    1.332126] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    1.332189] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    1.332252] Call Trace:
[    1.332281]  __platform_driver_probe+0x92/0xee
[    1.332323]  ? rtc_dev_init+0x2b/0x2b
[    1.332358]  cmos_init+0x37/0x67
[    1.332396]  do_one_initcall+0x7d/0x168
[    1.332428]  kernel_init_freeable+0x16c/0x1c9
[    1.332473]  ? rest_init+0xc0/0xc0
[    1.332508]  kernel_init+0x5/0x100
[    1.332543]  ret_from_fork+0x1f/0x30
[    1.332579] CR2: 00000000000000c8
[    1.332616] ---[ end trace 3bd87f12e9010b87 ]---
[    1.333549] note: swapper/0[1] exited with preempt_count 1
[    1.333592] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
[    1.333736] Kernel Offset: disabled

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/base/platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b5ce7b085795..3e75c4893789 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -830,6 +830,8 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv,
 	/* temporary section violation during probe() */
 	drv->probe = probe;
 	retval = code = __platform_driver_register(drv, module);
+	if (retval)
+		return retval;
 
 	/*
 	 * Fixup that section violation, being paranoid about code scanning
-- 
2.17.1

Re: [PATCH v1 1/1] drivers: base: Fix NULL pointer exception in __platform_driver_probe()

[Greg KH]

On Wed, Apr 08, 2020 at 02:40:03PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> If platform bus driver registration is failed then,

How is your platform driver registration failing?  What caused that, is
it an in-kernel problem with something?  How can it be triggered?

thanks,

greg k-h

Re: [PATCH v1 1/1] drivers: base: Fix NULL pointer exception in __platform_driver_probe()

[Kuppuswamy, Sathyanarayanan]

Hi Greg,

On 4/28/20 12:03 PM, Greg KH wrote:
> On Wed, Apr 08, 2020 at 02:40:03PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> If platform bus driver registration is failed then,
> 
> How is your platform driver registration failing?  What caused that, is
> it an in-kernel problem with something?  How can it be triggered?
In my case I triggered it intentionally. For one of our internal
project we want to strictly control the number of drivers/devices
allowed in kernel. To verify the feasibility of adding above support,
I intentionally failed few bus drivers and checked the behavior. In
one of those tests I hence came across the mentioned issue.

In any case, platform bus driver registration failure is a valid case
right ? Any issue we notice when this happens needs to be handled right?
> 
> thanks,
> 
> greg k-h
> 

Re: [PATCH v1 1/1] drivers: base: Fix NULL pointer exception in __platform_driver_probe()

[Greg KH]

On Tue, Apr 28, 2020 at 12:24:05PM -0700, Kuppuswamy, Sathyanarayanan wrote:
> Hi Greg,
> 
> On 4/28/20 12:03 PM, Greg KH wrote:
> > On Wed, Apr 08, 2020 at 02:40:03PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > > 
> > > If platform bus driver registration is failed then,
> > 
> > How is your platform driver registration failing?  What caused that, is
> > it an in-kernel problem with something?  How can it be triggered?
> In my case I triggered it intentionally. For one of our internal
> project we want to strictly control the number of drivers/devices
> allowed in kernel. To verify the feasibility of adding above support,
> I intentionally failed few bus drivers and checked the behavior. In
> one of those tests I hence came across the mentioned issue.
> 
> In any case, platform bus driver registration failure is a valid case
> right ? Any issue we notice when this happens needs to be handled right?

That's fine, I just need to know if this is something that someone can
actually trigger today, and needs to be fixed up, or if this is just a
"hardening for when a driver author does something foolish" type of a
case.

It seems to be the "don't do foolish things" to me, so it's a much lower
priority as we can always fix foolish drivers because we have the source
to them :)

thanks,

greg k-h