From patchwork Tue Jan 30 16:59:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Limonciello, Mario" X-Patchwork-Id: 10192057 X-Patchwork-Delegate: andy.shevchenko@gmail.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 05CAD6020C for ; Tue, 30 Jan 2018 16:59:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9C7220952 for ; Tue, 30 Jan 2018 16:59:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DD754209CE; Tue, 30 Jan 2018 16:59:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0E79520952 for ; Tue, 30 Jan 2018 16:59:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753535AbeA3Q7N (ORCPT ); Tue, 30 Jan 2018 11:59:13 -0500 Received: from esa6.dell-outbound.iphmx.com ([68.232.149.229]:36181 "EHLO esa6.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751559AbeA3Q7L (ORCPT ); Tue, 30 Jan 2018 11:59:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=dell.com; i=@dell.com; q=dns/txt; s=smtpout; t=1517331551; x=1548867551; h=from:to:cc:subject:date:message-id; bh=zIePG05Z/Ila//B0cPind87SQ/xk62zXzv0SrAbbNIY=; b=tDq6rfEZ0M803+TZTRIalRTAHIUmPsT0YknZhTEddOEWqYZkZBJlLU21 aUKet2deuanlICt6VwD4CsJ7G8x2ck1CmPD/iNPvZadMCKTb4F18r7lDU KL2KaVZbxmPbGhGCUvj1IshLo6jJRIBEhRpugQkqr+G5HYLRJP9Vvp2KP I=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A2EzAABzo3BamGOa6ERbGwEBAQEDAQEBC?= =?us-ascii?q?QEBAYUdKJxxixSQSgqFO4JKWBQBAQEBAQEBAQIBAhABAQEBAQgLCwYoL4I4JAG?= =?us-ascii?q?CdVJPMzwBEoodAxWoKzqHOw2DHTKEV4IVgVeIAUQEh2sFimSIcJAGPYJKjhiFB?= =?us-ascii?q?gKUJQKOLYk6gTw2gXNwgnyCBQFPEAyCJVmObQEBAQ?= X-IPAS-Result: =?us-ascii?q?A2EzAABzo3BamGOa6ERbGwEBAQEDAQEBCQEBAYUdKJxxixS?= =?us-ascii?q?QSgqFO4JKWBQBAQEBAQEBAQIBAhABAQEBAQgLCwYoL4I4JAGCdVJPMzwBEoodA?= =?us-ascii?q?xWoKzqHOw2DHTKEV4IVgVeIAUQEh2sFimSIcJAGPYJKjhiFBgKUJQKOLYk6gTw?= =?us-ascii?q?2gXNwgnyCBQFPEAyCJVmObQEBAQ?= Received: from esa6.dell-outbound2.iphmx.com ([68.232.154.99]) by esa6.dell-outbound.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jan 2018 10:59:10 -0600 Received: from ausc60ps301.us.dell.com ([143.166.148.206]) by esa6.dell-outbound2.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jan 2018 22:59:10 +0600 X-LoopCount0: from 10.208.86.39 X-IronPort-AV: E=Sophos;i="5.46,435,1511848800"; d="scan'208";a="1091263163" X-DLP: DLP_GlobalPCIDSS From: Mario Limonciello To: dvhart@infradead.org, Andy Shevchenko Cc: pali.rohar@gmail.com, LKML , platform-driver-x86@vger.kernel.org, Mario Limonciello Subject: [PATCH] platform/x86: dell-laptop: Allocate buffer on heap rather than globally Date: Tue, 30 Jan 2018 10:59:00 -0600 Message-Id: <1517331540-13266-1-git-send-email-mario.limonciello@dell.com> X-Mailer: git-send-email 2.7.4 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 There may be race conditions with multiple different functions working on a module wide buffer causing one function to get the wrong results. Suggested-by: Pali Rohar Signed-off-by: Mario Limonciello --- drivers/platform/x86/dell-laptop.c | 188 ++++++++++++++++++++----------------- 1 file changed, 103 insertions(+), 85 deletions(-) diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index fc2dfc8..ab58373 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -78,7 +78,6 @@ static struct platform_driver platform_driver = { } }; -static struct calling_interface_buffer *buffer; static struct platform_device *platform_device; static struct backlight_device *dell_backlight_device; static struct rfkill *wifi_rfkill; @@ -322,7 +321,8 @@ static const struct dmi_system_id dell_quirks[] __initconst = { { } }; -static void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3) +static void dell_set_arguments(struct calling_interface_buffer *buffer, + u32 arg0, u32 arg1, u32 arg2, u32 arg3) { memset(buffer, 0, sizeof(struct calling_interface_buffer)); buffer->input[0] = arg0; @@ -331,7 +331,8 @@ static void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3) buffer->input[3] = arg3; } -static int dell_send_request(u16 class, u16 select) +static int dell_send_request(struct calling_interface_buffer *buffer, + u16 class, u16 select) { int ret; @@ -468,21 +469,22 @@ static int dell_rfkill_set(void *data, bool blocked) int disable = blocked ? 1 : 0; unsigned long radio = (unsigned long)data; int hwswitch_bit = (unsigned long)data - 1; + struct calling_interface_buffer buffer; int hwswitch; int status; int ret; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); if (ret) return ret; - status = buffer->output[1]; + status = buffer.output[1]; - dell_set_arguments(0x2, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 0x2, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); if (ret) return ret; - hwswitch = buffer->output[1]; + hwswitch = buffer.output[1]; /* If the hardware switch controls this radio, and the hardware switch is disabled, always disable the radio */ @@ -490,8 +492,8 @@ static int dell_rfkill_set(void *data, bool blocked) (status & BIT(0)) && !(status & BIT(16))) disable = 1; - dell_set_arguments(1 | (radio<<8) | (disable << 16), 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 1 | (radio<<8) | (disable << 16), 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); return ret; } @@ -500,9 +502,11 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio, { if (status & BIT(0)) { /* Has hw-switch, sync sw_state to BIOS */ + struct calling_interface_buffer buffer; int block = rfkill_blocked(rfkill); - dell_set_arguments(1 | (radio << 8) | (block << 16), 0, 0, 0); - dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, + 1 | (radio << 8) | (block << 16), 0, 0, 0); + dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); } else { /* No hw-switch, sync BIOS state to sw_state */ rfkill_set_sw_state(rfkill, !!(status & BIT(radio + 16))); @@ -519,21 +523,22 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio, static void dell_rfkill_query(struct rfkill *rfkill, void *data) { int radio = ((unsigned long)data & 0xF); + struct calling_interface_buffer buffer; int hwswitch; int status; int ret; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); - status = buffer->output[1]; + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); + status = buffer.output[1]; if (ret != 0 || !(status & BIT(0))) { return; } - dell_set_arguments(0, 0x2, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); - hwswitch = buffer->output[1]; + dell_set_arguments(&buffer, 0, 0x2, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); + hwswitch = buffer.output[1]; if (ret != 0) return; @@ -550,22 +555,23 @@ static struct dentry *dell_laptop_dir; static int dell_debugfs_show(struct seq_file *s, void *data) { + struct calling_interface_buffer buffer; int hwswitch_state; int hwswitch_ret; int status; int ret; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); if (ret) return ret; - status = buffer->output[1]; + status = buffer.output[1]; - dell_set_arguments(0, 0x2, 0, 0); - hwswitch_ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 0, 0x2, 0, 0); + hwswitch_ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); if (hwswitch_ret) return hwswitch_ret; - hwswitch_state = buffer->output[1]; + hwswitch_state = buffer.output[1]; seq_printf(s, "return:\t%d\n", ret); seq_printf(s, "status:\t0x%X\n", status); @@ -646,22 +652,23 @@ static const struct file_operations dell_debugfs_fops = { static void dell_update_rfkill(struct work_struct *ignored) { + struct calling_interface_buffer buffer; int hwswitch = 0; int status; int ret; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); - status = buffer->output[1]; + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); + status = buffer.output[1]; if (ret != 0) return; - dell_set_arguments(0, 0x2, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); + dell_set_arguments(&buffer, 0, 0x2, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); if (ret == 0 && (status & BIT(0))) - hwswitch = buffer->output[1]; + hwswitch = buffer.output[1]; if (wifi_rfkill) { dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch); @@ -719,6 +726,7 @@ static struct notifier_block dell_laptop_rbtn_notifier = { static int __init dell_setup_rfkill(void) { + struct calling_interface_buffer buffer; int status, ret, whitelisted; const char *product; @@ -734,9 +742,9 @@ static int __init dell_setup_rfkill(void) if (!force_rfkill && !whitelisted) return 0; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL); - status = buffer->output[1]; + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); + status = buffer.output[1]; /* dell wireless info smbios call is not supported */ if (ret != 0) @@ -889,6 +897,7 @@ static void dell_cleanup_rfkill(void) static int dell_send_intensity(struct backlight_device *bd) { + struct calling_interface_buffer buffer; struct calling_interface_token *token; int ret; @@ -896,17 +905,21 @@ static int dell_send_intensity(struct backlight_device *bd) if (!token) return -ENODEV; - dell_set_arguments(token->location, bd->props.brightness, 0, 0); + dell_set_arguments(&buffer, + token->location, bd->props.brightness, 0, 0); if (power_supply_is_system_supplied() > 0) - ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_AC); + ret = dell_send_request(&buffer, + CLASS_TOKEN_WRITE, SELECT_TOKEN_AC); else - ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_BAT); + ret = dell_send_request(&buffer, + CLASS_TOKEN_WRITE, SELECT_TOKEN_BAT); return ret; } static int dell_get_intensity(struct backlight_device *bd) { + struct calling_interface_buffer buffer; struct calling_interface_token *token; int ret; @@ -914,14 +927,17 @@ static int dell_get_intensity(struct backlight_device *bd) if (!token) return -ENODEV; - dell_set_arguments(token->location, 0, 0, 0); + dell_set_arguments(&buffer, token->location, 0, 0, 0); if (power_supply_is_system_supplied() > 0) - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC); + ret = dell_send_request(&buffer, + CLASS_TOKEN_READ, SELECT_TOKEN_AC); else - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_BAT); + ret = dell_send_request(&buffer, + CLASS_TOKEN_READ, SELECT_TOKEN_BAT); if (ret == 0) - ret = buffer->output[1]; + ret = buffer.output[1]; + return ret; } @@ -1186,31 +1202,33 @@ static enum led_brightness kbd_led_level; static int kbd_get_info(struct kbd_info *info) { + struct calling_interface_buffer buffer; u8 units; int ret; - dell_set_arguments(0, 0, 0, 0); - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); if (ret) return ret; - info->modes = buffer->output[1] & 0xFFFF; - info->type = (buffer->output[1] >> 24) & 0xFF; - info->triggers = buffer->output[2] & 0xFF; - units = (buffer->output[2] >> 8) & 0xFF; - info->levels = (buffer->output[2] >> 16) & 0xFF; + info->modes = buffer.output[1] & 0xFFFF; + info->type = (buffer.output[1] >> 24) & 0xFF; + info->triggers = buffer.output[2] & 0xFF; + units = (buffer.output[2] >> 8) & 0xFF; + info->levels = (buffer.output[2] >> 16) & 0xFF; if (quirks && quirks->kbd_led_levels_off_1 && info->levels) info->levels--; if (units & BIT(0)) - info->seconds = (buffer->output[3] >> 0) & 0xFF; + info->seconds = (buffer.output[3] >> 0) & 0xFF; if (units & BIT(1)) - info->minutes = (buffer->output[3] >> 8) & 0xFF; + info->minutes = (buffer.output[3] >> 8) & 0xFF; if (units & BIT(2)) - info->hours = (buffer->output[3] >> 16) & 0xFF; + info->hours = (buffer.output[3] >> 16) & 0xFF; if (units & BIT(3)) - info->days = (buffer->output[3] >> 24) & 0xFF; + info->days = (buffer.output[3] >> 24) & 0xFF; return ret; } @@ -1270,31 +1288,34 @@ static int kbd_set_level(struct kbd_state *state, u8 level) static int kbd_get_state(struct kbd_state *state) { + struct calling_interface_buffer buffer; int ret; - dell_set_arguments(0x1, 0, 0, 0); - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); + dell_set_arguments(&buffer, 0, 0, 0, 0); + ret = dell_send_request(&buffer, + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); if (ret) return ret; - state->mode_bit = ffs(buffer->output[1] & 0xFFFF); + state->mode_bit = ffs(buffer.output[1] & 0xFFFF); if (state->mode_bit != 0) state->mode_bit--; - state->triggers = (buffer->output[1] >> 16) & 0xFF; - state->timeout_value = (buffer->output[1] >> 24) & 0x3F; - state->timeout_unit = (buffer->output[1] >> 30) & 0x3; - state->als_setting = buffer->output[2] & 0xFF; - state->als_value = (buffer->output[2] >> 8) & 0xFF; - state->level = (buffer->output[2] >> 16) & 0xFF; - state->timeout_value_ac = (buffer->output[2] >> 24) & 0x3F; - state->timeout_unit_ac = (buffer->output[2] >> 30) & 0x3; + state->triggers = (buffer.output[1] >> 16) & 0xFF; + state->timeout_value = (buffer.output[1] >> 24) & 0x3F; + state->timeout_unit = (buffer.output[1] >> 30) & 0x3; + state->als_setting = buffer.output[2] & 0xFF; + state->als_value = (buffer.output[2] >> 8) & 0xFF; + state->level = (buffer.output[2] >> 16) & 0xFF; + state->timeout_value_ac = (buffer.output[2] >> 24) & 0x3F; + state->timeout_unit_ac = (buffer.output[2] >> 30) & 0x3; return ret; } static int kbd_set_state(struct kbd_state *state) { + struct calling_interface_buffer buffer; int ret; u32 input1; u32 input2; @@ -1307,8 +1328,9 @@ static int kbd_set_state(struct kbd_state *state) input2 |= (state->level & 0xFF) << 16; input2 |= (state->timeout_value_ac & 0x3F) << 24; input2 |= (state->timeout_unit_ac & 0x3) << 30; - dell_set_arguments(0x2, input1, input2, 0); - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); + dell_set_arguments(&buffer, 0x2, input1, input2, 0); + ret = dell_send_request(&buffer, + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT); return ret; } @@ -1335,6 +1357,7 @@ static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old) static int kbd_set_token_bit(u8 bit) { + struct calling_interface_buffer buffer; struct calling_interface_token *token; int ret; @@ -1345,14 +1368,15 @@ static int kbd_set_token_bit(u8 bit) if (!token) return -EINVAL; - dell_set_arguments(token->location, token->value, 0, 0); - ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD); + dell_set_arguments(&buffer, token->location, token->value, 0, 0); + ret = dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD); return ret; } static int kbd_get_token_bit(u8 bit) { + struct calling_interface_buffer buffer; struct calling_interface_token *token; int ret; int val; @@ -1364,9 +1388,9 @@ static int kbd_get_token_bit(u8 bit) if (!token) return -EINVAL; - dell_set_arguments(token->location, 0, 0, 0); - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_STD); - val = buffer->output[1]; + dell_set_arguments(&buffer, token->location, 0, 0, 0); + ret = dell_send_request(&buffer, CLASS_TOKEN_READ, SELECT_TOKEN_STD); + val = buffer.output[1]; if (ret) return ret; @@ -2102,6 +2126,7 @@ static struct notifier_block dell_laptop_notifier = { int dell_micmute_led_set(int state) { + struct calling_interface_buffer buffer; struct calling_interface_token *token; if (state == 0) @@ -2114,8 +2139,8 @@ int dell_micmute_led_set(int state) if (!token) return -ENODEV; - dell_set_arguments(token->location, token->value, 0, 0); - dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD); + dell_set_arguments(&buffer, token->location, token->value, 0, 0); + dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD); return state; } @@ -2146,13 +2171,6 @@ static int __init dell_init(void) if (ret) goto fail_platform_device2; - buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL); - if (!buffer) { - ret = -ENOMEM; - goto fail_buffer; - } - - ret = dell_setup_rfkill(); if (ret) { @@ -2177,10 +2195,13 @@ static int __init dell_init(void) token = dell_smbios_find_token(BRIGHTNESS_TOKEN); if (token) { - dell_set_arguments(token->location, 0, 0, 0); - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC); + struct calling_interface_buffer buffer; + + dell_set_arguments(&buffer, token->location, 0, 0, 0); + ret = dell_send_request(&buffer, + CLASS_TOKEN_READ, SELECT_TOKEN_AC); if (ret) - max_intensity = buffer->output[3]; + max_intensity = buffer.output[3]; } if (max_intensity) { @@ -2214,8 +2235,6 @@ static int __init dell_init(void) fail_get_brightness: backlight_device_unregister(dell_backlight_device); fail_backlight: - kfree(buffer); -fail_buffer: dell_cleanup_rfkill(); fail_rfkill: platform_device_del(platform_device); @@ -2235,7 +2254,6 @@ static void __exit dell_exit(void) touchpad_led_exit(); kbd_led_exit(); backlight_device_unregister(dell_backlight_device); - kfree(buffer); dell_cleanup_rfkill(); if (platform_device) { platform_device_unregister(platform_device);