All of lore.kernel.org
 help / color / mirror / Atom feed
* [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	[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.