[PATCH v2] dell-laptop: Check return value of all SMBIOS calls and do not cache hwswitch state

[Pali RohÃr]

Make sure that return value of all SMBIOS calls are properly checked and
do not continue of processing (received) information if call failed.

Also do not chache hwswitch wireless state as it can be changed at runtime
(e.g from userspace smbios applications).

Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
---
Changes since v1:
 * Call clear_buffer before each sequential SMBIOS call (we expect zero-filled buffer)
 * Do not cache hwswitch state as it can be modified at runtime by userspace
 * simplify some conditions
---
 drivers/platform/x86/dell-laptop.c |  173 ++++++++++++++++++++++++++----------
 1 file changed, 127 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 35758cb..99f28d3 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -308,12 +308,15 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 static struct calling_interface_buffer *buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
-static int hwswitch_state;
+static void clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
 
 static void get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	clear_buffer();
 }
 
 static void release_buffer(void)
@@ -547,21 +550,41 @@ 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;
+	int hwswitch;
+	int status;
+	int ret;
 
 	get_buffer();
+
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	status = buffer->output[1];
+
+	if (ret != 0)
+		goto out;
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
-	if ((hwswitch_state & BIT(hwswitch_bit)) &&
-	    !(buffer->output[1] & BIT(16)))
+	if (ret == 0 && (hwswitch & BIT(hwswitch_bit)) &&
+	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
+	clear_buffer();
+
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 
+ out:
 	release_buffer();
-	return 0;
+	return dell_smi_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -571,6 +594,7 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 	if (status & BIT(0)) {
 		/* Has hw-switch, sync sw_state to BIOS */
 		int block = rfkill_blocked(rfkill);
+		clear_buffer();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
 		dell_send_request(buffer, 17, 11);
 	} else {
@@ -580,23 +604,43 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 }
 
 static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
-					int status)
+					int status, int hwswitch)
 {
-	if (hwswitch_state & (BIT(radio - 1)))
+	if (hwswitch & (BIT(radio - 1)))
 		rfkill_set_hw_state(rfkill, !(status & BIT(16)));
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
+	int radio = ((unsigned long)data & 0xF);
+	int hwswitch;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	if (ret != 0 || !(status & BIT(0))) {
+		release_buffer();
+		return;
+	}
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
 
 	release_buffer();
+
+	if (ret != 0)
+		return;
+
+	dell_rfkill_update_hw_state(rfkill, radio, status, hwswitch);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -608,13 +652,27 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
+	int hwswitch_state;
+	int hwswitch_ret;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	hwswitch_ret = buffer->output[0];
+	hwswitch_state = buffer->output[1];
+
 	release_buffer();
 
+	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
 		   status & BIT(0));
@@ -656,7 +714,9 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	seq_printf(s, "Bit 21: WiGig is blocked:            %lu\n",
 		  (status & BIT(21)) >> 21);
 
-	seq_printf(s, "\nhwswitch_state:\t0x%X\n", hwswitch_state);
+	seq_printf(s, "\n");
+	seq_printf(s, "hwswitch_return:\t%d\n", hwswitch_ret);
+	seq_printf(s, "hwswitch_state:\t0x%X\n", hwswitch_state);
 	seq_printf(s, "Bit 0 : Wifi controlled by switch:      %lu\n",
 		   hwswitch_state & BIT(0));
 	seq_printf(s, "Bit 1 : Bluetooth controlled by switch: %lu\n",
@@ -692,25 +752,44 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
+	int hwswitch;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
+	if (ret != 0)
+		goto out;
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret == 0 && (status & BIT(0)))
+		hwswitch = buffer->output[1];
+	else
+		hwswitch = 0;
+
 	if (wifi_rfkill) {
-		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
+		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
 	}
 	if (bluetooth_rfkill) {
-		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status);
+		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status, hwswitch);
 		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
 	}
 	if (wwan_rfkill) {
-		dell_rfkill_update_hw_state(wwan_rfkill, 3, status);
+		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
+ out:
 	release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
@@ -772,21 +851,17 @@ static int __init dell_setup_rfkill(void)
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
-	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
-	hwswitch_state = buffer->output[1];
 	release_buffer();
 
-	if (!(status & BIT(0))) {
-		if (force_rfkill) {
-			/* No hwsitch, clear all hw-controlled bits */
-			hwswitch_state &= ~7;
-		} else {
-			/* rfkill is only tested on laptops with a hwswitch */
-			return 0;
-		}
-	}
+	/* dell wireless info smbios call is not supported */
+	if (ret != 0)
+		return 0;
+
+	/* rfkill is only tested on laptops with a hwswitch */
+	if (!(status & BIT(0)) && !force_rfkill)
+		return 0;
 
 	if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
 		wifi_rfkill = rfkill_alloc("dell-wifi", &platform_device->dev,
@@ -931,47 +1006,50 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int ret;
+	int token;
+
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
-
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 1, 2);
 	else
 		dell_send_request(buffer, 1, 1);
 
- out:
+	ret = dell_smi_error(buffer->output[0]);
+
 	release_buffer();
 	return ret;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int ret;
+	int token;
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
+	get_buffer();
+	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 0, 2);
 	else
 		dell_send_request(buffer, 0, 1);
 
-	ret = buffer->output[1];
+	if (buffer->output[0])
+		ret = dell_smi_error(buffer->output[0]);
+	else
+		ret = buffer->output[1];
 
- out:
 	release_buffer();
 	return ret;
 }
@@ -2035,6 +2113,7 @@ static void kbd_led_exit(void)
 static int __init dell_init(void)
 {
 	int max_intensity = 0;
+	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2098,13 +2177,15 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	if (buffer->input[0] != -1) {
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token != -1) {
+		get_buffer();
+		buffer->input[0] = token;
 		dell_send_request(buffer, 0, 2);
-		max_intensity = buffer->output[3];
+		if (buffer->output[0] == 0)
+			max_intensity = buffer->output[3];
+		release_buffer();
 	}
-	release_buffer();
 
 	if (max_intensity) {
 		struct backlight_properties props;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/