* [PATCH] usb: gadget: u_serial: Fix possible null pointer dereference
@ 2021-09-19 19:57 Florian Faber
2021-09-20 4:57 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Florian Faber @ 2021-09-19 19:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb
In gs_rx_push, tty can be null. tty_throttled accesses tty->flags without further testing, which would lead to a null pointer dereference.
Signed-off-by: Florian Faber <faber@faberman.de>
---
drivers/usb/gadget/function/u_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 6f68cbeeee7c..00512e7c7261 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -436,7 +436,7 @@ static void gs_rx_push(struct work_struct *work)
* We may leave non-empty queue only when there is a tty, and
* either it is throttled or there is no more room in flip buffer.
*/
- if (!list_empty(queue) && !tty_throttled(tty))
+ if (tty && !list_empty(queue) && !tty_throttled(tty))
schedule_delayed_work(&port->push, 1);
/* If we're still connected, refill the USB RX queue. */
--
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: u_serial: Fix possible null pointer dereference
2021-09-19 19:57 [PATCH] usb: gadget: u_serial: Fix possible null pointer dereference Florian Faber
@ 2021-09-20 4:57 ` Greg KH
2021-09-20 8:36 ` Florian Faber
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2021-09-20 4:57 UTC (permalink / raw)
To: Florian Faber; +Cc: linux-usb
On Sun, Sep 19, 2021 at 09:57:28PM +0200, Florian Faber wrote:
> In gs_rx_push, tty can be null. tty_throttled accesses tty->flags without further testing, which would lead to a null pointer dereference.
Please wrap your changelog text to 72 columns.
>
> Signed-off-by: Florian Faber <faber@faberman.de>
> ---
> drivers/usb/gadget/function/u_serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index 6f68cbeeee7c..00512e7c7261 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -436,7 +436,7 @@ static void gs_rx_push(struct work_struct *work)
> * We may leave non-empty queue only when there is a tty, and
> * either it is throttled or there is no more room in flip buffer.
> */
> - if (!list_empty(queue) && !tty_throttled(tty))
> + if (tty && !list_empty(queue) && !tty_throttled(tty))
Have you ever been able to trigger this with a NULL tty?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: u_serial: Fix possible null pointer dereference
2021-09-20 4:57 ` Greg KH
@ 2021-09-20 8:36 ` Florian Faber
0 siblings, 0 replies; 3+ messages in thread
From: Florian Faber @ 2021-09-20 8:36 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb
Greg,
On 9/20/21 6:57 AM, Greg KH wrote:
> On Sun, Sep 19, 2021 at 09:57:28PM +0200, Florian Faber wrote:
>> In gs_rx_push, tty can be null. tty_throttled accesses tty->flags without further testing, which would lead to a null pointer dereference.
>
> Please wrap your changelog text to 72 columns.
>
>>
>> Signed-off-by: Florian Faber <faber@faberman.de>
>> ---
>> drivers/usb/gadget/function/u_serial.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
>> index 6f68cbeeee7c..00512e7c7261 100644
>> --- a/drivers/usb/gadget/function/u_serial.c
>> +++ b/drivers/usb/gadget/function/u_serial.c
>> @@ -436,7 +436,7 @@ static void gs_rx_push(struct work_struct *work)
>> * We may leave non-empty queue only when there is a tty, and
>> * either it is throttled or there is no more room in flip buffer.
>> */
>> - if (!list_empty(queue) && !tty_throttled(tty))
>> + if (tty && !list_empty(queue) && !tty_throttled(tty))
>
> Have you ever been able to trigger this with a NULL tty?
No, not practical. I've just stumbled across it.
70 lines above it is checked in the same way:
--------------------------snip------------------------------
/* leave data queued if tty was rx throttled */
if (tty && tty_throttled(tty))
--------------------------snip------------------------------
..and 50 lines above:
--------------------------snip------------------------------
/* push data to (open) tty */
if (req->actual && tty) {
--------------------------snip------------------------------
The check makes sense since tty CAN be null (depending on the local connection)
and it doesn't change its value after the first assignment.
Flo
--
Machines can do the work, so people have time to think.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-20 8:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-19 19:57 [PATCH] usb: gadget: u_serial: Fix possible null pointer dereference Florian Faber
2021-09-20 4:57 ` Greg KH
2021-09-20 8:36 ` Florian Faber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.