linux-kernel - [PATCH 1/2 v2] xhci: fix garbage USBSTS being logged in some cases

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: <20220225102602.3829106-1-anssi.hannula@bitwise.fi>
Date:   Fri, 25 Feb 2022 12:26:02 +0200
From:   Anssi Hannula <anssi.hannula@...wise.fi>
To:     Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH 1/2 v2] xhci: fix garbage USBSTS being logged in some cases

xhci_decode_usbsts() is expected to return a zero-terminated string by
its only caller, xhci_stop_endpoint_command_watchdog(), which directly
logs the return value:

  xhci_warn(xhci, "USBSTS:%s\n", xhci_decode_usbsts(str, usbsts));

However, if no recognized bits are set in usbsts, the function will
return without having called any sprintf() and therefore return an
untouched non-zero-terminated caller-provided buffer, causing garbage
to be output to log.

Fix that by always including the raw value in the output.

Note that before 4843b4b5ec64 ("xhci: fix even more unsafe memory usage
in xhci tracing") the result effect in the failure case was different as
a static buffer was used here, but the code still worked incorrectly.

Fixes: 9c1aa36efdae ("xhci: Show host status when watchdog triggers and host is assumed dead.")
Signed-off-by: Anssi Hannula <anssi.hannula@...wise.fi>
---

Mathias Nyman wrote:
> Maybe this could be the first thing printed out, something like (untested):
[...]

Heh, that's actually pretty close to what I had at one point, not sure
why I didn't go with it. I agree it looks better.

Changed and tested on real HW:

[   11.998832] xhci-hcd xhci-hcd.1.auto: xHCI host not responding to stop endpoint command.
[   12.006925] xhci-hcd xhci-hcd.1.auto: USBSTS: 0x00000000

v2: Improve print format on Mathias Nyman's suggestion.

 drivers/usb/host/xhci.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8a0026ee9524..dd24c09927bd 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2622,8 +2622,11 @@ static inline const char *xhci_decode_usbsts(char *str, u32 usbsts)
 {
 	int ret = 0;

+	ret = sprintf(str, " 0x%08x", usbsts);
+
 	if (usbsts == ~(u32)0)
-		return " 0xffffffff";
+		return str;
+
 	if (usbsts & STS_HALT)
 		ret += sprintf(str + ret, " HCHalted");
 	if (usbsts & STS_FATAL)
-- 
2.34.1