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-next>] [day] [month] [year] [list]
Message-Id: <20241015-hid-fix-fetch_item-unreachable-v1-1-b131cd10dbd1@kernel.org>
Date: Tue, 15 Oct 2024 12:23:47 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>, 
 Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, 
 llvm@...ts.linux.dev, patches@...ts.linux.dev, 
 "Paul E. McKenney" <paulmck@...nel.org>, 
 Nathan Chancellor <nathan@...nel.org>
Subject: [PATCH] HID: Remove default case statement in fetch_item()

A default case statement with a bare unreachable() was recently added to
fetch_item(), which by itself introduces undefined behavior. objtool
points this out with a few different warnings, depending on
configuration and compiler:

  vmlinux.o: warning: objtool: fetch_item() falls through to next function ...

  vmlinux.o: warning: objtool: hid_open_report() falls through to next function hid_parser_main()
  vmlinux.o: warning: objtool: hid_scan_report() falls through to next function hid_allocate_device()

  vmlinux.o: warning: objtool: hid_open_report+0x21b: can't find jump dest instruction at .text.hid_open_report+0x40f

Replacing unreachable() with BUG() is a typical fix to eliminate the
undefined behavior and make the default case well defined. However, in
this case, all possible values are enumerated in the switch statement,
so the default case can never actually happen, as proven with the
comment next to the item->size assignment. Just remove the default case
altogether, as the return statement would still be valid if the switch
statement were ever to be skipped.

Fixes: 61595012f280 ("HID: simplify code in fetch_item()")
Suggested-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
Closes: https://lore.kernel.org/20241010222451.GA3571761@thelio-3990X/
Reported-by: Paul E. McKenney <paulmck@...nel.org>
Closes: https://lore.kernel.org/fe8c909e-bf02-4466-b3eb-0a4747df32e3@paulmck-laptop/
Tested-by: Paul E. McKenney <paulmck@...nel.org>
Signed-off-by: Nathan Chancellor <nathan@...nel.org>
---
 drivers/hid/hid-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 77725e33592098a0bd45222cfafc4b7c80daae54..3e3166d5719490afe88530d0e5aec3d63a96ed55 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -818,9 +818,6 @@ static const u8 *fetch_item(const __u8 *start, const __u8 *end, struct hid_item
 	case 4:
 		item->data.u32 = get_unaligned_le32(start);
 		break;
-
-	default:
-		unreachable();
 	}
 
 	return start + item->size;

---
base-commit: af27f2c22f5e3dc61e787f1b1d9f4b3cddf4af25
change-id: 20241015-hid-fix-fetch_item-unreachable-9c05547c856d

Best regards,
-- 
Nathan Chancellor <nathan@...nel.org>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ