linux-kernel - [PATCH 02/12] HID: core: de-duplicate some code in hid_input

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220126161832.3193805-3-benjamin.tissoires@redhat.com>
Date:   Wed, 26 Jan 2022 17:18:22 +0100
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Jiri Kosina <jikos@...nel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Ahelenia Ziemiańska 
        <nabijaczleweli@...ijaczleweli.xyz>,
        Ping Cheng <pinglinux@...il.com>,
        Aaron Armstrong Skomra <skomra@...il.com>,
        Jason Gerecke <killertofu@...il.com>,
        Peter Hutterer <peter.hutterer@...-t.net>
Cc:     linux-input@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>
Subject: [PATCH 02/12] HID: core: de-duplicate some code in hid_input_field()

I had to go twice through the history to get a grasp at this code.
De-duplicate the various tests in one common helper to make it
more explicit.

Note that the `HID_UP_KEYBOARD + 1` condition is tested through
https://gitlab.freedesktop.org/libevdev/hid-tools/-/merge_requests/121

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
---
 drivers/hid/hid-core.c | 54 ++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 75e7b8447bf7..fad4dbdf6391 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1525,6 +1525,24 @@ static void hid_process_event(struct hid_device *hid, struct hid_field *field,
 		hid->hiddev_hid_event(hid, field, usage, value);
 }
 
+/*
+ * Checks if the given value is valid within this field
+ */
+static inline int hid_array_value_is_valid(struct hid_field *field,
+					   __s32 value)
+{
+	__s32 min = field->logical_minimum;
+
+	/*
+	 * Value needs to be between logical min and max, and
+	 * (value - min) is used as an index in the usage array.
+	 * This array is of size field->maxusage
+	 */
+	return value >= min &&
+	       value <= field->logical_maximum &&
+	       value - min < field->maxusage;
+}
+
 /*
  * Analyse a received field, and fetch the data from it. The field
  * content is stored for next report processing (we do differential
@@ -1539,7 +1557,6 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 	unsigned offset = field->report_offset;
 	unsigned size = field->report_size;
 	__s32 min = field->logical_minimum;
-	__s32 max = field->logical_maximum;
 	__s32 *value;
 
 	value = field->new_value;
@@ -1554,8 +1571,7 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 
 		/* Ignore report if ErrorRollOver */
 		if (!(field->flags & HID_MAIN_ITEM_VARIABLE) &&
-		    value[n] >= min && value[n] <= max &&
-		    value[n] - min < field->maxusage &&
+		    hid_array_value_is_valid(field, value[n]) &&
 		    field->usage[value[n] - min].hid == HID_UP_KEYBOARD + 1)
 			return;
 	}
@@ -1563,21 +1579,29 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
 	for (n = 0; n < count; n++) {
 
 		if (HID_MAIN_ITEM_VARIABLE & field->flags) {
-			hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
+			hid_process_event(hid,
+					  field,
+					  &field->usage[n],
+					  value[n],
+					  interrupt);
 			continue;
 		}
 
-		if (field->value[n] >= min && field->value[n] <= max
-			&& field->value[n] - min < field->maxusage
-			&& field->usage[field->value[n] - min].hid
-			&& search(value, field->value[n], count))
-				hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
-
-		if (value[n] >= min && value[n] <= max
-			&& value[n] - min < field->maxusage
-			&& field->usage[value[n] - min].hid
-			&& search(field->value, value[n], count))
-				hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt);
+		if (hid_array_value_is_valid(field, field->value[n]) &&
+		    search(value, field->value[n], count))
+			hid_process_event(hid,
+					  field,
+					  &field->usage[field->value[n] - min],
+					  0,
+					  interrupt);
+
+		if (hid_array_value_is_valid(field, value[n]) &&
+		    search(field->value, value[n], count))
+			hid_process_event(hid,
+					  field,
+					  &field->usage[value[n] - min],
+					  1,
+					  interrupt);
 	}
 
 	memcpy(field->value, value, count * sizeof(__s32));
-- 
2.33.1