| From b5fdf5c6e6bee35837e160c00ac89327bdad031b Mon Sep 17 00:00:00 2001 |
| From: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> |
| Date: Fri, 25 Jun 2021 15:14:56 +1200 |
| Subject: usb: max-3421: Prevent corruption of freed memory |
| |
| From: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> |
| |
| commit b5fdf5c6e6bee35837e160c00ac89327bdad031b upstream. |
| |
| The MAX-3421 USB driver remembers the state of the USB toggles for a |
| device/endpoint. To save SPI writes, this was only done when a new |
| device/endpoint was being used. Unfortunately, if the old device was |
| removed, this would cause writes to freed memory. |
| |
| To fix this, a simpler scheme is used. The toggles are read from |
| hardware when a URB is completed, and the toggles are always written to |
| hardware when any URB transaction is started. This will cause a few more |
| SPI transactions, but no causes kernel panics. |
| |
| Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.") |
| Cc: stable <stable@vger.kernel.org> |
| Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz> |
| Link: https://lore.kernel.org/r/20210625031456.8632-1-mark.tomlinson@alliedtelesis.co.nz |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/usb/host/max3421-hcd.c | 44 +++++++++++++---------------------------- |
| 1 file changed, 14 insertions(+), 30 deletions(-) |
| |
| --- a/drivers/usb/host/max3421-hcd.c |
| +++ b/drivers/usb/host/max3421-hcd.c |
| @@ -153,8 +153,6 @@ struct max3421_hcd { |
| */ |
| struct urb *curr_urb; |
| enum scheduling_pass sched_pass; |
| - struct usb_device *loaded_dev; /* dev that's loaded into the chip */ |
| - int loaded_epnum; /* epnum whose toggles are loaded */ |
| int urb_done; /* > 0 -> no errors, < 0: errno */ |
| size_t curr_len; |
| u8 hien; |
| @@ -492,39 +490,17 @@ max3421_set_speed(struct usb_hcd *hcd, s |
| * Caller must NOT hold HCD spinlock. |
| */ |
| static void |
| -max3421_set_address(struct usb_hcd *hcd, struct usb_device *dev, int epnum, |
| - int force_toggles) |
| +max3421_set_address(struct usb_hcd *hcd, struct usb_device *dev, int epnum) |
| { |
| - struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd); |
| - int old_epnum, same_ep, rcvtog, sndtog; |
| - struct usb_device *old_dev; |
| + int rcvtog, sndtog; |
| u8 hctl; |
| |
| - old_dev = max3421_hcd->loaded_dev; |
| - old_epnum = max3421_hcd->loaded_epnum; |
| - |
| - same_ep = (dev == old_dev && epnum == old_epnum); |
| - if (same_ep && !force_toggles) |
| - return; |
| - |
| - if (old_dev && !same_ep) { |
| - /* save the old end-points toggles: */ |
| - u8 hrsl = spi_rd8(hcd, MAX3421_REG_HRSL); |
| - |
| - rcvtog = (hrsl >> MAX3421_HRSL_RCVTOGRD_BIT) & 1; |
| - sndtog = (hrsl >> MAX3421_HRSL_SNDTOGRD_BIT) & 1; |
| - |
| - /* no locking: HCD (i.e., we) own toggles, don't we? */ |
| - usb_settoggle(old_dev, old_epnum, 0, rcvtog); |
| - usb_settoggle(old_dev, old_epnum, 1, sndtog); |
| - } |
| /* setup new endpoint's toggle bits: */ |
| rcvtog = usb_gettoggle(dev, epnum, 0); |
| sndtog = usb_gettoggle(dev, epnum, 1); |
| hctl = (BIT(rcvtog + MAX3421_HCTL_RCVTOG0_BIT) | |
| BIT(sndtog + MAX3421_HCTL_SNDTOG0_BIT)); |
| |
| - max3421_hcd->loaded_epnum = epnum; |
| spi_wr8(hcd, MAX3421_REG_HCTL, hctl); |
| |
| /* |
| @@ -532,7 +508,6 @@ max3421_set_address(struct usb_hcd *hcd, |
| * address-assignment so it's best to just always load the |
| * address whenever the end-point changed/was forced. |
| */ |
| - max3421_hcd->loaded_dev = dev; |
| spi_wr8(hcd, MAX3421_REG_PERADDR, dev->devnum); |
| } |
| |
| @@ -667,7 +642,7 @@ max3421_select_and_start_urb(struct usb_ |
| struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd); |
| struct urb *urb, *curr_urb = NULL; |
| struct max3421_ep *max3421_ep; |
| - int epnum, force_toggles = 0; |
| + int epnum; |
| struct usb_host_endpoint *ep; |
| struct list_head *pos; |
| unsigned long flags; |
| @@ -777,7 +752,6 @@ done: |
| usb_settoggle(urb->dev, epnum, 0, 1); |
| usb_settoggle(urb->dev, epnum, 1, 1); |
| max3421_ep->pkt_state = PKT_STATE_SETUP; |
| - force_toggles = 1; |
| } else |
| max3421_ep->pkt_state = PKT_STATE_TRANSFER; |
| } |
| @@ -785,7 +759,7 @@ done: |
| spin_unlock_irqrestore(&max3421_hcd->lock, flags); |
| |
| max3421_ep->last_active = max3421_hcd->frame_number; |
| - max3421_set_address(hcd, urb->dev, epnum, force_toggles); |
| + max3421_set_address(hcd, urb->dev, epnum); |
| max3421_set_speed(hcd, urb->dev); |
| max3421_next_transfer(hcd, 0); |
| return 1; |
| @@ -1380,6 +1354,16 @@ max3421_urb_done(struct usb_hcd *hcd) |
| status = 0; |
| urb = max3421_hcd->curr_urb; |
| if (urb) { |
| + /* save the old end-points toggles: */ |
| + u8 hrsl = spi_rd8(hcd, MAX3421_REG_HRSL); |
| + int rcvtog = (hrsl >> MAX3421_HRSL_RCVTOGRD_BIT) & 1; |
| + int sndtog = (hrsl >> MAX3421_HRSL_SNDTOGRD_BIT) & 1; |
| + int epnum = usb_endpoint_num(&urb->ep->desc); |
| + |
| + /* no locking: HCD (i.e., we) own toggles, don't we? */ |
| + usb_settoggle(urb->dev, epnum, 0, rcvtog); |
| + usb_settoggle(urb->dev, epnum, 1, sndtog); |
| + |
| max3421_hcd->curr_urb = NULL; |
| spin_lock_irqsave(&max3421_hcd->lock, flags); |
| usb_hcd_unlink_urb_from_ep(hcd, urb); |