From patchwork Mon Feb 15 16:32:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Lutomirski X-Patchwork-Id: 8316671 Return-Path: X-Original-To: patchwork-platform-driver-x86@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D55929F372 for ; Mon, 15 Feb 2016 16:32:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E966E203B4 for ; Mon, 15 Feb 2016 16:32:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 47FC7203C2 for ; Mon, 15 Feb 2016 16:32:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751367AbcBOQcn (ORCPT ); Mon, 15 Feb 2016 11:32:43 -0500 Received: from mail.kernel.org ([198.145.29.136]:34119 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751303AbcBOQcn (ORCPT ); Mon, 15 Feb 2016 11:32:43 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EE8CF20361; Mon, 15 Feb 2016 16:32:41 +0000 (UTC) Received: from localhost (c-67-166-60-92.hsd1.co.comcast.net [67.166.60.92]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 05166203C2; Mon, 15 Feb 2016 16:32:40 +0000 (UTC) From: Andy Lutomirski To: =?UTF-8?q?Pali=20Roh=C3=A1r?= , platform-driver-x86@vger.kernel.org, Darren Hart Cc: Jon Eyolfson , Matthew Garrett , Mario Limonciello , Andy Lutomirski Subject: [PATCH v5 1/5] dell-wmi: Stop storing pointers to DMI tables Date: Mon, 15 Feb 2016 08:32:33 -0800 Message-Id: <384e9535556e09169a16c4d6e28e419322ecf26f.1455553470.git.luto@kernel.org> X-Mailer: git-send-email 2.5.0 In-Reply-To: References: In-Reply-To: References: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The dmi_walk function maps the DMI table, walks it, and unmaps it. This means that the dell_bios_hotkey_table that find_hk_type stores points to unmapped memory by the time it gets read. I've been able to trigger crashes caused by the stale pointer a couple of times, but never on a stock kernel. Fix it by generating the keymap in the dmi_walk callback instead of storing a pointer. Signed-off-by: Andy Lutomirski --- Changes from v3: None Changes from v2: - dmi_walk failure is no longer fatal (Jean) Changes from v1: - Rename handle_dmi_table to handle_dmi_entry (Jean) - Remove useless assignment to results->err (Jean) drivers/platform/x86/dell-wmi.c | 74 +++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 368e193c2741..d6ae69e0a787 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -120,7 +120,10 @@ struct dell_bios_hotkey_table { }; -static const struct dell_bios_hotkey_table *dell_bios_hotkey_table; +struct dell_dmi_results { + int err; + struct key_entry *keymap; +}; /* Uninitialized entries here are KEY_RESERVED == 0. */ static const u16 bios_to_linux_keycode[256] __initconst = { @@ -337,20 +340,34 @@ static void dell_wmi_notify(u32 value, void *context) kfree(obj); } -static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) +static void __init handle_dmi_entry(const struct dmi_header *dm, + void *opaque) { - int hotkey_num = (dell_bios_hotkey_table->header.length - 4) / - sizeof(struct dell_bios_keymap_entry); + struct dell_dmi_results *results = opaque; + struct dell_bios_hotkey_table *table; struct key_entry *keymap; - int i; + int hotkey_num, i; + + if (results->err || results->keymap) + return; /* We already found the hotkey table. */ + + if (dm->type != 0xb2 || dm->length <= 6) + return; + + table = container_of(dm, struct dell_bios_hotkey_table, header); + + hotkey_num = (table->header.length - 4) / + sizeof(struct dell_bios_keymap_entry); keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL); - if (!keymap) - return NULL; + if (!keymap) { + results->err = -ENOMEM; + return; + } for (i = 0; i < hotkey_num; i++) { const struct dell_bios_keymap_entry *bios_entry = - &dell_bios_hotkey_table->keymap[i]; + &table->keymap[i]; /* Uninitialized entries are 0 aka KEY_RESERVED. */ u16 keycode = (bios_entry->keycode < @@ -379,11 +396,12 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) keymap[hotkey_num].type = KE_END; - return keymap; + results->keymap = keymap; } static int __init dell_wmi_input_setup(void) { + struct dell_dmi_results dmi_results = {}; int err; dell_wmi_input_dev = input_allocate_device(); @@ -394,20 +412,31 @@ static int __init dell_wmi_input_setup(void) dell_wmi_input_dev->phys = "wmi/input0"; dell_wmi_input_dev->id.bustype = BUS_HOST; - if (dell_new_hk_type) { - const struct key_entry *keymap = dell_wmi_prepare_new_keymap(); - if (!keymap) { - err = -ENOMEM; - goto err_free_dev; - } + if (dmi_walk(handle_dmi_entry, &dmi_results)) { + /* + * Historically, dell-wmi ignored dmi_walk errors. A failure + * is certainly surprising, but it probably just indicates + * a very old laptop. + */ + pr_warn("no DMI; using the old-style hotkey interface\n"); + } + + if (dmi_results.err) { + err = dmi_results.err; + goto err_free_dev; + } + + if (dmi_results.keymap) { + dell_new_hk_type = true; - err = sparse_keymap_setup(dell_wmi_input_dev, keymap, NULL); + err = sparse_keymap_setup(dell_wmi_input_dev, + dmi_results.keymap, NULL); /* * Sparse keymap library makes a copy of keymap so we * don't need the original one that was allocated. */ - kfree(keymap); + kfree(dmi_results.keymap); } else { err = sparse_keymap_setup(dell_wmi_input_dev, dell_wmi_legacy_keymap, NULL); @@ -434,15 +463,6 @@ static void dell_wmi_input_destroy(void) input_unregister_device(dell_wmi_input_dev); } -static void __init find_hk_type(const struct dmi_header *dm, void *dummy) -{ - if (dm->type == 0xb2 && dm->length > 6) { - dell_new_hk_type = true; - dell_bios_hotkey_table = - container_of(dm, struct dell_bios_hotkey_table, header); - } -} - /* * Descriptor buffer is 128 byte long and contains: * @@ -524,8 +544,6 @@ static int __init dell_wmi_init(void) if (err) return err; - dmi_walk(find_hk_type, NULL); - err = dell_wmi_input_setup(); if (err) return err;