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>] [day] [month] [year] [list]
Message-Id: <20191218082920.3095-1-baijiaju1990@gmail.com>
Date:   Wed, 18 Dec 2019 16:29:20 +0800
From:   Jia-Ju Bai <baijiaju1990@...il.com>
To:     jikos@...nel.org, benjamin.tissoires@...hat.com
Cc:     linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jia-Ju Bai <baijiaju1990@...il.com>
Subject: [PATCH] hid: hid-lg4ff: fix possible sleep-in-atomic-context bug

The driver may sleep while holding a read lock.
The function call path (from bottom to top) in Linux 4.19 is:

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 538: 
	hid_hw_request in lg4ff_set_autocenter_default
drivers/hid/hid-lg4ff.c, 497: 
	_raw_spin_lock_irqsave in lg4ff_set_autocenter_default

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 585: 
	hid_hw_request in lg4ff_set_autocenter_ffex
drivers/hid/hid-lg4ff.c, 576: 
	_raw_spin_lock_irqsave in lg4ff_set_autocenter_ffex

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 1116: 
	hid_hw_request in lg4ff_set_leds
drivers/hid/hid-lg4ff.c, 1108: 
	_raw_spin_lock_irqsave in lg4ff_set_leds

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 465: 
	hid_hw_request in lg4ff_play
drivers/hid/hid-lg4ff.c, 441: 
	_raw_spin_lock_irqsave in lg4ff_play

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 664: 
	hid_hw_request in lg4ff_set_range_dfp
drivers/hid/hid-lg4ff.c, 648: 
	_raw_spin_lock_irqsave in lg4ff_set_range_dfp

drivers/hid/hid-core.c, 1459: 
	hid_alloc_report_buf(GFP_KERNEL) in __hid_request
./include/linux/hid.h, 1051: 
	__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 620: 
	hid_hw_request in lg4ff_set_range_g25
drivers/hid/hid-lg4ff.c, 611: 
	_raw_spin_lock_irqsave in lg4ff_set_range_g25

hid_alloc_report_buf(GFP_KERNEL) can sleep at runtime.

To fix these bugs, hid_hw_request() is called without holding the
spinlock.

These bugs are found by a static analysis tool STCheck written by myself.

Signed-off-by: Jia-Ju Bai <baijiaju1990@...il.com>
---
 drivers/hid/hid-lg4ff.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 5e6a0cef2a06..047a19da3aed 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -443,8 +443,8 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 			value[5] = 0x00;
 			value[6] = 0x00;
 
-			hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 			spin_unlock_irqrestore(&entry->report_lock, flags);
+			hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 			return 0;
 		}
 
@@ -456,8 +456,8 @@ static int lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effec
 		value[5] = 0x00;
 		value[6] = 0x00;
 
-		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		break;
 	}
 	return 0;
@@ -498,8 +498,8 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 		value[5] = 0x00;
 		value[6] = 0x00;
 
-		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		return;
 	}
 
@@ -529,7 +529,9 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
+	spin_lock_irqsave(&entry->report_lock, flags);
 
 	/* Activate Auto-Center */
 	value[0] = 0x14;
@@ -540,8 +542,8 @@ static void lg4ff_set_autocenter_default(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 }
 
 /* Sends autocentering command compatible with Formula Force EX */
@@ -576,8 +578,8 @@ static void lg4ff_set_autocenter_ffex(struct input_dev *dev, u16 magnitude)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 }
 
 /* Sends command to set range compatible with G25/G27/Driving Force GT */
@@ -611,8 +613,8 @@ static void lg4ff_set_range_g25(struct hid_device *hid, u16 range)
 	value[5] = 0x00;
 	value[6] = 0x00;
 
-	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 }
 
 /* Sends commands to set range compatible with Driving Force Pro wheel */
@@ -655,7 +657,9 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 		value[1] = 0x02;
 		full_range = 200;
 	}
+	spin_unlock_irqrestore(&entry->report_lock, flags);
 	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
+	spin_lock_irqsave(&entry->report_lock, flags);
 
 	/* Prepare "fine" limit command */
 	value[0] = 0x81;
@@ -667,8 +671,8 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[6] = 0x00;
 
 	if (range == 200 || range == 900) {	/* Do not apply any fine limit */
-		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		spin_unlock_irqrestore(&entry->report_lock, flags);
+		hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 		return;
 	}
 
@@ -682,8 +686,8 @@ static void lg4ff_set_range_dfp(struct hid_device *hid, u16 range)
 	value[5] = (start_right & 0xe) << 4 | (start_left & 0xe);
 	value[6] = 0xff;
 
-	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 }
 
 static const struct lg4ff_compat_mode_switch *lg4ff_get_mode_switch_command(const u16 real_product_id, const u16 target_product_id)
@@ -1107,8 +1111,8 @@ static void lg4ff_set_leds(struct hid_device *hid, u8 leds)
 	value[4] = 0x00;
 	value[5] = 0x00;
 	value[6] = 0x00;
-	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 	spin_unlock_irqrestore(&entry->report_lock, flags);
+	hid_hw_request(hid, entry->report, HID_REQ_SET_REPORT);
 }
 
 static void lg4ff_led_set_brightness(struct led_classdev *led_cdev,
-- 
2.17.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ