patch "usb: max-3421: Prevent corruption of freed memory" added to usb-linus
Subsystems (2)
This is a note to let you know that I've just added the patch titled
usb: max-3421: Prevent corruption of freed memory
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From b5fdf5c6e6bee35837e160c00ac89327bdad031b Mon Sep 17 00:00:00 2001
From: Mark Tomlinson <[email protected]>
Date: Fri, 25 Jun 2021 15:14:56 +1200
Subject: 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.
--- drivers/usb/host/max3421-hcd.c | 44 +++++++++++----------------------- 1 file changed, 14 insertions(+), 30 deletions(-)
drivers/usb/host/max3421-hcd.c +14 -30
@@ -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 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)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 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)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);
Signature · 1 line
2.32.0