* [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend
@ 2023-05-05 9:18 Prashanth K
2023-05-06 15:45 ` Alan Stern
0 siblings, 1 reply; 5+ messages in thread
From: Prashanth K @ 2023-05-05 9:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern
Cc: Xiu Jianfeng, Christophe JAILLET, linux-usb, linux-kernel, Prashanth K
Consider a case where gserial_disconnect has already cleared
gser->ioport. And if gserial_suspend gets called afterwards,
it will lead to accessing of gser->ioport and thus causing
null pointer dereference.
Avoid this by adding a null pointer check. Added a static
spinlock to prevent gser->ioport from becoming null after
the newly added null pointer check.
Fixes: aba3a8d01d62 ("usb: gadget: u_serial: add suspend resume callbacks")
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
---
drivers/usb/gadget/function/u_serial.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index a0ca47f..e5d522d 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1420,10 +1420,19 @@ 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);
}
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend
2023-05-05 9:18 [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend Prashanth K
@ 2023-05-06 15:45 ` Alan Stern
0 siblings, 0 replies; 5+ messages in thread
From: Alan Stern @ 2023-05-06 15:45 UTC (permalink / raw)
To: Prashanth K
Cc: Greg Kroah-Hartman, Xiu Jianfeng, Christophe JAILLET, linux-usb,
linux-kernel
On Fri, May 05, 2023 at 02:48:37PM +0530, Prashanth K wrote:
> Consider a case where gserial_disconnect has already cleared
> gser->ioport. And if gserial_suspend gets called afterwards,
> it will lead to accessing of gser->ioport and thus causing
> null pointer dereference.
>
> Avoid this by adding a null pointer check. Added a static
> spinlock to prevent gser->ioport from becoming null after
> the newly added null pointer check.
>
> Fixes: aba3a8d01d62 ("usb: gadget: u_serial: add suspend resume callbacks")
> Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
> ---
> drivers/usb/gadget/function/u_serial.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index a0ca47f..e5d522d 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -1420,10 +1420,19 @@ 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);
> }
This looks fine to me, but I'm not a serial-gadget maintainer.
In fact, it looks like we don't have a serial-gadget maintainer.
Alan Stern
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend
@ 2023-05-22 2:21 Chunfeng Yun
2023-05-22 5:49 ` Prashanth K
0 siblings, 1 reply; 5+ messages in thread
From: Chunfeng Yun @ 2023-05-22 2:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Alan Stern,
Chunfeng Yun, Prashanth K, Xiu Jianfeng, Christophe JAILLET,
Fabrice Gasnier, Felipe Balbi, linux-usb, linux-kernel,
linux-arm-kernel, linux-mediatek, Kewu Chen, stable
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);
}
--
2.18.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend
2023-05-22 2:21 Chunfeng Yun
@ 2023-05-22 5:49 ` Prashanth K
2023-05-25 7:00 ` Chunfeng Yun (云春峰)
0 siblings, 1 reply; 5+ messages in thread
From: Prashanth K @ 2023-05-22 5:49 UTC (permalink / raw)
To: Chunfeng Yun, Greg Kroah-Hartman
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Alan Stern,
Xiu Jianfeng, Christophe JAILLET, Fabrice Gasnier, Felipe Balbi,
linux-usb, linux-kernel, linux-arm-kernel, linux-mediatek,
Kewu Chen, stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend
2023-05-22 5:49 ` Prashanth K
@ 2023-05-25 7:00 ` Chunfeng Yun (云春峰)
0 siblings, 0 replies; 5+ messages in thread
From: Chunfeng Yun (云春峰) @ 2023-05-25 7:00 UTC (permalink / raw)
To: quic_prashk, gregkh
Cc: linux-kernel, linux-mediatek, linux-usb, fabrice.gasnier, stable,
balbi, xiujianfeng, Kewu Chen (陈科伍),
stern, linux-arm-kernel, matthias.bgg, christophe.jaillet,
angelogioacchino.delregno
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-25 7:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-05 9:18 [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_suspend Prashanth K
2023-05-06 15:45 ` Alan Stern
2023-05-22 2:21 Chunfeng Yun
2023-05-22 5:49 ` Prashanth K
2023-05-25 7:00 ` Chunfeng Yun (云春峰)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®