| Message ID | 20240202154217.661867-2-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5fd9e45f1ebcd57181358af28506e8a661a260b3 |
| Headers | show |
| Series | [v2,1/2] usb: cdns3: fixed memory use after free at cdns3_gadget_ep_disable() | expand |
On 24-02-02 10:42:17, Frank Li wrote: > 829 if (request->complete) { > 830 spin_unlock(&priv_dev->lock); > 831 usb_gadget_giveback_request(&priv_ep->endpoint, > 832 request); > 833 spin_lock(&priv_dev->lock); > 834 } > 835 > 836 if (request->buf == priv_dev->zlp_buf) > 837 cdns3_gadget_ep_free_request(&priv_ep->endpoint, request); > > Driver append an additional zero packet request when queue a packet, which > length mod max packet size is 0. When transfer complete, run to line 831, > usb_gadget_giveback_request() will free this requestion. 836 condition is > true, so cdns3_gadget_ep_free_request() free this request again. > > Log: > > [ 1920.140696][ T150] BUG: KFENCE: use-after-free read in cdns3_gadget_giveback+0x134/0x2c0 [cdns3] > [ 1920.140696][ T150] > [ 1920.151837][ T150] Use-after-free read at 0x000000003d1cd10b (in kfence-#36): > [ 1920.159082][ T150] cdns3_gadget_giveback+0x134/0x2c0 [cdns3] > [ 1920.164988][ T150] cdns3_transfer_completed+0x438/0x5f8 [cdns3] > > Add check at line 829, skip call usb_gadget_giveback_request() if it is > additional zero length packet request. Needn't call > usb_gadget_giveback_request() because it is allocated in this driver. > > Cc: <stable@vger.kernel.org> > Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") > Signed-off-by: Frank Li <Frank.Li@nxp.com> Acked-by: Peter Chen <peter.chen@kernel.org> > --- > drivers/usb/cdns3/cdns3-gadget.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c > index d6723d31fc6e2..fd1beb10bba72 100644 > --- a/drivers/usb/cdns3/cdns3-gadget.c > +++ b/drivers/usb/cdns3/cdns3-gadget.c > @@ -828,7 +828,11 @@ void cdns3_gadget_giveback(struct cdns3_endpoint *priv_ep, > return; > } > > - if (request->complete) { > + /* > + * zlp request is appended by driver, needn't call usb_gadget_giveback_request() to notify > + * gadget composite driver. > + */ > + if (request->complete && request->buf != priv_dev->zlp_buf) { > spin_unlock(&priv_dev->lock); > usb_gadget_giveback_request(&priv_ep->endpoint, > request); > -- > 2.34.1 >
On 02/02/2024 17:42, Frank Li wrote: > 829 if (request->complete) { > 830 spin_unlock(&priv_dev->lock); > 831 usb_gadget_giveback_request(&priv_ep->endpoint, > 832 request); > 833 spin_lock(&priv_dev->lock); > 834 } > 835 > 836 if (request->buf == priv_dev->zlp_buf) > 837 cdns3_gadget_ep_free_request(&priv_ep->endpoint, request); > > Driver append an additional zero packet request when queue a packet, which > length mod max packet size is 0. When transfer complete, run to line 831, > usb_gadget_giveback_request() will free this requestion. 836 condition is > true, so cdns3_gadget_ep_free_request() free this request again. > > Log: > > [ 1920.140696][ T150] BUG: KFENCE: use-after-free read in cdns3_gadget_giveback+0x134/0x2c0 [cdns3] > [ 1920.140696][ T150] > [ 1920.151837][ T150] Use-after-free read at 0x000000003d1cd10b (in kfence-#36): > [ 1920.159082][ T150] cdns3_gadget_giveback+0x134/0x2c0 [cdns3] > [ 1920.164988][ T150] cdns3_transfer_completed+0x438/0x5f8 [cdns3] > > Add check at line 829, skip call usb_gadget_giveback_request() if it is > additional zero length packet request. Needn't call > usb_gadget_giveback_request() because it is allocated in this driver. > > Cc: <stable@vger.kernel.org> > Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") > Signed-off-by: Frank Li <Frank.Li@nxp.com> Reviewed-by: Roger Quadros <rogerq@kernel.org>
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index d6723d31fc6e2..fd1beb10bba72 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -828,7 +828,11 @@ void cdns3_gadget_giveback(struct cdns3_endpoint *priv_ep, return; } - if (request->complete) { + /* + * zlp request is appended by driver, needn't call usb_gadget_giveback_request() to notify + * gadget composite driver. + */ + if (request->complete && request->buf != priv_dev->zlp_buf) { spin_unlock(&priv_dev->lock); usb_gadget_giveback_request(&priv_ep->endpoint, request);
829 if (request->complete) { 830 spin_unlock(&priv_dev->lock); 831 usb_gadget_giveback_request(&priv_ep->endpoint, 832 request); 833 spin_lock(&priv_dev->lock); 834 } 835 836 if (request->buf == priv_dev->zlp_buf) 837 cdns3_gadget_ep_free_request(&priv_ep->endpoint, request); Driver append an additional zero packet request when queue a packet, which length mod max packet size is 0. When transfer complete, run to line 831, usb_gadget_giveback_request() will free this requestion. 836 condition is true, so cdns3_gadget_ep_free_request() free this request again. Log: [ 1920.140696][ T150] BUG: KFENCE: use-after-free read in cdns3_gadget_giveback+0x134/0x2c0 [cdns3] [ 1920.140696][ T150] [ 1920.151837][ T150] Use-after-free read at 0x000000003d1cd10b (in kfence-#36): [ 1920.159082][ T150] cdns3_gadget_giveback+0x134/0x2c0 [cdns3] [ 1920.164988][ T150] cdns3_transfer_completed+0x438/0x5f8 [cdns3] Add check at line 829, skip call usb_gadget_giveback_request() if it is additional zero length packet request. Needn't call usb_gadget_giveback_request() because it is allocated in this driver. Cc: <stable@vger.kernel.org> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/usb/cdns3/cdns3-gadget.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)