lkml.org 
[lkml]   [2021]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.13 164/223] usb: max-3421: Prevent corruption of freed memory
    Date
    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);

    \
     
     \ /
      Last update: 2021-07-26 18:44    [W:4.114 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site