| Message ID | 20220222134117.34844-1-anssi.hannula@bitwise.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] xhci: fix garbage USBSTS being logged in some cases | expand |
On 22.2.2022 15.41, Anssi Hannula wrote: > 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@bitwise.fi> > --- > > Noticed this while debugging a USB issue. Let me know if you prefer a > different fix. > > drivers/usb/host/xhci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 8a0026ee9524..ac91647195f6 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -2642,6 +2642,7 @@ static inline const char *xhci_decode_usbsts(char *str, u32 usbsts) > ret += sprintf(str + ret, " CNR"); > if (usbsts & STS_HCE) > ret += sprintf(str + ret, " HCE"); > + ret += sprintf(str + ret, " (0x%08x)", usbsts); Thanks, nice catch. Maybe this could be the first thing printed out, something like (untested): @@ -2697,8 +2697,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; + -Mathias
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8a0026ee9524..ac91647195f6 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -2642,6 +2642,7 @@ static inline const char *xhci_decode_usbsts(char *str, u32 usbsts) ret += sprintf(str + ret, " CNR"); if (usbsts & STS_HCE) ret += sprintf(str + ret, " HCE"); + ret += sprintf(str + ret, " (0x%08x)", usbsts); return str; }
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@bitwise.fi> --- Noticed this while debugging a USB issue. Let me know if you prefer a different fix. drivers/usb/host/xhci.h | 1 + 1 file changed, 1 insertion(+)