diff mbox series

[3/9] xhci: fix uninitialized string returned by xhci_decode_ctrl_ctx()

Message ID 20220303102656.1661407-4-mathias.nyman@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series xhci cleanups and fixes for usb-next | expand

Commit Message

Mathias Nyman March 3, 2022, 10:26 a.m. UTC
From: Anssi Hannula <anssi.hannula@bitwise.fi>

xhci_decode_ctrl_ctx() returns the untouched buffer as-is if both "drop"
and "add" parameters are zero.

Fix the function to return an empty string in that case.

It was not immediately clear from the possible call chains whether this
issue is currently actually triggerable or not.

Note that before commit 4843b4b5ec64 ("xhci: fix even more unsafe memory
Cc: stable@vger.kernel.org
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: 90d6d5731da7 ("xhci: Add tracing for input control context")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

commit 4843b4b5ec64 ("xhci: fix even more unsafe memory usage in xhci tracing")
---
 drivers/usb/host/xhci.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Anssi Hannula March 3, 2022, 10:43 a.m. UTC | #1
Hi,

On 3.3.2022 12.26, Mathias Nyman wrote:
> From: Anssi Hannula <anssi.hannula@bitwise.fi>
>
> xhci_decode_ctrl_ctx() returns the untouched buffer as-is if both "drop"
> and "add" parameters are zero.
>
> Fix the function to return an empty string in that case.
>
> It was not immediately clear from the possible call chains whether this
> issue is currently actually triggerable or not.
>
> Note that before commit 4843b4b5ec64 ("xhci: fix even more unsafe memory
> Cc: stable@vger.kernel.org
> 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.

You added the Cc-stable line a few lines too early above :)

> Fixes: 90d6d5731da7 ("xhci: Add tracing for input control context")
> Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>
> commit 4843b4b5ec64 ("xhci: fix even more unsafe memory usage in xhci tracing")
> ---
>  drivers/usb/host/xhci.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 1d83ddace482..473a33ce299e 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -2468,6 +2468,8 @@ static inline const char *xhci_decode_ctrl_ctx(char *str,
>  	unsigned int	bit;
>  	int		ret = 0;
>  
> +	str[0] = '\0';
> +
>  	if (drop) {
>  		ret = sprintf(str, "Drop:");
>  		for_each_set_bit(bit, &drop, 32)
Mathias Nyman March 3, 2022, 10:59 a.m. UTC | #2
On 3.3.2022 12.43, Anssi Hannula wrote:
> Hi,
> 
> On 3.3.2022 12.26, Mathias Nyman wrote:
>> From: Anssi Hannula <anssi.hannula@bitwise.fi>
>>
>> xhci_decode_ctrl_ctx() returns the untouched buffer as-is if both "drop"
>> and "add" parameters are zero.
>>
>> Fix the function to return an empty string in that case.
>>
>> It was not immediately clear from the possible call chains whether this
>> issue is currently actually triggerable or not.
>>
>> Note that before commit 4843b4b5ec64 ("xhci: fix even more unsafe memory
>> Cc: stable@vger.kernel.org
>> 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.
> 
> You added the Cc-stable line a few lines too early above :)

Oops, copypaste accident. 

I'll resubmit 

Thanks
-Mathias
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 1d83ddace482..473a33ce299e 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2468,6 +2468,8 @@  static inline const char *xhci_decode_ctrl_ctx(char *str,
 	unsigned int	bit;
 	int		ret = 0;
 
+	str[0] = '\0';
+
 	if (drop) {
 		ret = sprintf(str, "Drop:");
 		for_each_set_bit(bit, &drop, 32)