| Message ID | 20230522022101.32163-1-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: gadget: u_serial: Add null pointer check in gserial_suspend | expand |
On 22-05-23 07:51 am, Chunfeng Yun wrote: > When gserial_disconnect has already cleared gser->ioport, and the > suspend triggers afterwards, gserial_suspend gets called, which will > lead to accessing of gser->ioport and thus causing null pointer > dereference. Add a null pointer check to prevent it as the bellow > patch does: > 5ec63fdbca60 ("usb: gadget: u_serial: Add null pointer check in gserial_resume") > > Fixes: aba3a8d01d62 ("usb: gadget: u_serial: add suspend resume callbacks") > Cc: stable <stable@kernel.org> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/usb/gadget/function/u_serial.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c > index a0ca47fbff0f..40ba220cf6d2 100644 > --- a/drivers/usb/gadget/function/u_serial.c > +++ b/drivers/usb/gadget/function/u_serial.c > @@ -1420,10 +1420,18 @@ EXPORT_SYMBOL_GPL(gserial_disconnect); > > void gserial_suspend(struct gserial *gser) > { > - struct gs_port *port = gser->ioport; > + struct gs_port *port; > unsigned long flags; > > - spin_lock_irqsave(&port->port_lock, flags); > + spin_lock_irqsave(&serial_port_lock, flags); > + port = gser->ioport; > + if (!port) { > + spin_unlock_irqrestore(&serial_port_lock, flags); > + return; > + } > + > + spin_lock(&port->port_lock); > + spin_unlock(&serial_port_lock); > port->suspended = true; > spin_unlock_irqrestore(&port->port_lock, flags); > } Hi Chunfeng, This looks same as the following patch. https://lore.kernel.org/linux-usb/1683278317-11774-1-git-send-email-quic_prashk@quicinc.com/ Regards
On Mon, 2023-05-22 at 11:19 +0530, Prashanth K wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 22-05-23 07:51 am, Chunfeng Yun wrote: > > When gserial_disconnect has already cleared gser->ioport, and the > > suspend triggers afterwards, gserial_suspend gets called, which > > will > > lead to accessing of gser->ioport and thus causing null pointer > > dereference. Add a null pointer check to prevent it as the bellow > > patch does: > > 5ec63fdbca60 ("usb: gadget: u_serial: Add null pointer check in > > gserial_resume") > > > > Fixes: aba3a8d01d62 ("usb: gadget: u_serial: add suspend resume > > callbacks") > > Cc: stable <stable@kernel.org> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > --- > > drivers/usb/gadget/function/u_serial.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/gadget/function/u_serial.c > > b/drivers/usb/gadget/function/u_serial.c > > index a0ca47fbff0f..40ba220cf6d2 100644 > > --- a/drivers/usb/gadget/function/u_serial.c > > +++ b/drivers/usb/gadget/function/u_serial.c > > @@ -1420,10 +1420,18 @@ EXPORT_SYMBOL_GPL(gserial_disconnect); > > > > void gserial_suspend(struct gserial *gser) > > { > > - struct gs_port *port = gser->ioport; > > + struct gs_port *port; > > unsigned long flags; > > > > - spin_lock_irqsave(&port->port_lock, flags); > > + spin_lock_irqsave(&serial_port_lock, flags); > > + port = gser->ioport; > > + if (!port) { > > + spin_unlock_irqrestore(&serial_port_lock, flags); > > + return; > > + } > > + > > + spin_lock(&port->port_lock); > > + spin_unlock(&serial_port_lock); > > port->suspended = true; > > spin_unlock_irqrestore(&port->port_lock, flags); > > } > > Hi Chunfeng, > > This looks same as the following patch. > https://lore.kernel.org/linux-usb/1683278317-11774-1-git-send-email-quic_prashk@quicinc.com/ Yes, it is, please ignore this one, thanks a lot > > > Regards
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index a0ca47fbff0f..40ba220cf6d2 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1420,10 +1420,18 @@ EXPORT_SYMBOL_GPL(gserial_disconnect); void gserial_suspend(struct gserial *gser) { - struct gs_port *port = gser->ioport; + struct gs_port *port; unsigned long flags; - spin_lock_irqsave(&port->port_lock, flags); + spin_lock_irqsave(&serial_port_lock, flags); + port = gser->ioport; + if (!port) { + spin_unlock_irqrestore(&serial_port_lock, flags); + return; + } + + spin_lock(&port->port_lock); + spin_unlock(&serial_port_lock); port->suspended = true; spin_unlock_irqrestore(&port->port_lock, flags); }
When gserial_disconnect has already cleared gser->ioport, and the suspend triggers afterwards, gserial_suspend gets called, which will lead to accessing of gser->ioport and thus causing null pointer dereference. Add a null pointer check to prevent it as the bellow patch does: 5ec63fdbca60 ("usb: gadget: u_serial: Add null pointer check in gserial_resume") Fixes: aba3a8d01d62 ("usb: gadget: u_serial: add suspend resume callbacks") Cc: stable <stable@kernel.org> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/usb/gadget/function/u_serial.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)