usb: max-3421: Prevent corruption of freed memory

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: 2d53139f31 ("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>
This commit is contained in:
Mark Tomlinson 2021-06-25 15:14:56 +12:00 committed by Greg Kroah-Hartman
parent 5b01248156
commit b5fdf5c6e6
1 changed files with 14 additions and 30 deletions

View File

@ -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, struct usb_device *dev)
* 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, struct usb_device *dev, int epnum,
* 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_hcd *hcd)
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;
@ -1379,6 +1353,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);