linux-kernel - Re: [PATCH] ipmi: initialize platform driver even when using passed-in config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c34f6ed3-115a-d560-ddf9-023ba01d3679@acm.org>
Date:   Thu, 19 Jul 2018 20:41:44 -0500
From:   Corey Minyard <minyard@....org>
To:     Olof Johansson <olof@...om.net>
Cc:     openipmi-developer@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ipmi: initialize platform driver even when using
 passed-in config

On 07/06/2018 01:43 PM, Olof Johansson wrote:
> On platforms where manual values are passed in when the module is loaded
> (or through kernel command line), the device is instantiated with a
> reference to the platform driver, but the platform driver never gets
> initialized and loaded.

Sorry, I've been on vacation and busy on other stuff.

Your change is pretty simple, but it changes the behaviour of the 
driver.  Before, if
you had any hardcoded entries, it would ignore everything else. With 
your change
it will pick up platform entries.

I think the right thing to do is to create another platform driver for 
entries that have
no device (hardcode and hotmod, I think).  But I'm not 100% sure on this.

-corey

> Instead, always initialize the driver, but override any SMBIOS-found
> entries with the hardcoded values where they are in conflict.
>
> Crash is as below:
>
> [   20.052087] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
> [   20.067730] PGD 0
> [   20.071821] P4D 0
> [   20.075836] Oops: 0000 [#1] SMP PTI
> [   20.082864] CPU: 1 PID: 1330 Comm: systemd-udevd Not tainted 4.18.0-rc3-00134-g06c85639897c #10
> [   20.100394] Hardware name: <...>
> [   20.121583] RIP: 0010:sysfs_do_create_link_sd.isra.2+0x2f/0xa0
> [   20.361345] RSP: 0018:ffffc9000fab7b10 EFLAGS: 00010246
> [   20.371765] RAX: 0000000000000000 RBX: 0000000000000030 RCX: 0000000000000001
> [   20.386072] RDX: 0000000000000001 RSI: 0000000000000030 RDI: ffffffff82a8ef2c
> [   20.400312] RBP: ffffffff820dcdda R08: 0000000000000001 R09: 0000000000000044
> [   20.414560] R10: 0000000000000220 R11: ffff883ff80471e9 R12: ffff881fee1646e8
> [   20.428808] R13: 0000000000000001 R14: 0000000000000001 R15: 0000000000000000
> [   20.443056] FS:  00007f9d6c1e7940(0000) GS:ffff881ffff80000(0000) knlGS:0000000000000000
> [   20.459210] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   20.470685] CR2: 0000000000000030 CR3: 0000003fd1872006 CR4: 00000000007606e0
> [   20.484934] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   20.499182] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   20.513430] PKRU: 55555554
> [   20.518837] Call Trace:
> [   20.523729]  driver_sysfs_add+0x75/0xa0
> [   20.531388]  device_bind_driver+0xf/0x50
> [   20.539223]  __device_attach+0x89/0x100
> [   20.546885]  ? kobject_uevent_env+0x109/0x530
> [   20.555585]  bus_probe_device+0x8a/0xa0
> [   20.563245]  device_add+0x3df/0x590
> [   20.570214]  platform_device_add+0xb9/0x220
> [   20.578571]  try_smi_init+0x785/0x930 [ipmi_si]
> [   20.587618]  init_ipmi_si+0xfd/0x1a0 [ipmi_si]
> [   20.596493]  ? cleanup_ipmi_si+0x80/0x80 [ipmi_si]
> [   20.606060]  do_one_initcall+0x4e/0x1c9
> [   20.613722]  ? kmem_cache_alloc_trace+0x14e/0x1a0
> [   20.623118]  ? do_init_module+0x22/0x213
> [   20.630950]  do_init_module+0x5a/0x213
> [   20.638438]  load_module+0x1cef/0x2650
> [   20.645927]  ? m_show+0x1c0/0x1c0
> [   20.652550]  ? security_capable+0x3f/0x60
> [   20.660555]  __do_sys_finit_module+0xb2/0xc0
> [   20.669083]  do_syscall_64+0x49/0xf0
> [   20.676225]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Signed-off-by: Olof Johansson <olof@...om.net>
> ---
>   drivers/char/ipmi/ipmi_si_intf.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index dd758223c56d..6ddfc8be0c76 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -2018,7 +2018,13 @@ int ipmi_si_add_smi(struct si_sm_io *io)
>   	mutex_lock(&smi_infos_lock);
>   	dup = find_dup_si(new_smi);
>   	if (dup) {
> -		if (new_smi->io.addr_source == SI_ACPI &&
> +		if (new_smi->io.addr_source == SI_HARDCODED) {
> +			/* Assume the user knew what they wanted when they hardcoded */
> +			dev_info(dup->io.dev,
> +				"Removing discovered %s state machine in favor of hardcoded\n",
> +				si_to_str[new_smi->io.si_type]);
> +			cleanup_one_si(dup);
> +		} else if (new_smi->io.addr_source == SI_ACPI &&
>   		    dup->io.addr_source == SI_SMBIOS) {
>   			/* We prefer ACPI over SMBIOS. */
>   			dev_info(dup->io.dev,
> @@ -2341,12 +2347,13 @@ static int init_ipmi_si(void)
>   
>   	pr_info("IPMI System Interface driver.\n");
>   
> +	/* Always platform since hardcoded entries rely on it. */
> +	ipmi_si_platform_init();
> +
>   	/* If the user gave us a device, they presumably want us to use it */
>   	if (!ipmi_si_hardcode_find_bmc())
>   		goto do_scan;
>   
> -	ipmi_si_platform_init();
> -
>   	ipmi_si_pci_init();
>   
>   	ipmi_si_parisc_init();