| Message ID | 1394712342-15778-363-Taiwan-albertk@realtek.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 08997b5eec08a2c29367f19a74abdea54b299406 |
| Headers | show |
| Series | r8152: serial fixes | expand |
On Fri, May 21, 2021 at 05:07:34PM +0800, Hayes Wang wrote: > Verify some fields of the USB descriptor to make sure the driver > could be used by the device. > > BugLink: https://syzkaller.appspot.com/bug?id=912c9c373656996801b4de61f1e3cb326fe940aa > Reported-by: syzbot+95afd23673f5dd295c57@syzkaller.appspotmail.com > Fixes: c2198943e33b ("r8152: search the configuration of vendor mode") > Signed-off-by: Hayes Wang <hayeswang@realtek.com> > --- > drivers/net/usb/r8152.c | 71 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > index 136ea06540ff..f348350f5da1 100644 > --- a/drivers/net/usb/r8152.c > +++ b/drivers/net/usb/r8152.c > @@ -8107,6 +8107,69 @@ static void r8156b_init(struct r8152 *tp) > tp->coalesce = 15000; /* 15 us */ > } > > +static bool rtl_check_vendor_ok(struct usb_interface *intf) > +{ > + struct usb_host_interface *alt = intf->cur_altsetting; > + struct usb_host_endpoint *in = NULL, *out = NULL, *intr = NULL; > + unsigned int ep; > + > + if (alt->desc.bNumEndpoints < 3) { > + dev_err(&intf->dev, "Unexpected bNumEndpoints %d\n", alt->desc.bNumEndpoints); > + return false; > + } > + > + for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) { > + struct usb_host_endpoint *e; > + > + e = alt->endpoint + ep; > + > + /* ignore endpoints which cannot transfer data */ > + if (!usb_endpoint_maxp(&e->desc)) > + continue; > + > + switch (e->desc.bmAttributes) { > + case USB_ENDPOINT_XFER_INT: > + if (!usb_endpoint_dir_in(&e->desc)) > + continue; > + if (!intr) > + intr = e; > + break; > + case USB_ENDPOINT_XFER_BULK: > + if (usb_endpoint_dir_in(&e->desc)) { > + if (!in) > + in = e; > + } else if (!out) { > + out = e; > + } > + break; > + default: > + continue; > + } > + } > + > + if (!in || !out || !intr) { > + dev_err(&intf->dev, "Miss Endpoints\n"); > + return false; > + } > + > + if ((in->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 1) { > + dev_err(&intf->dev, "Invalid Rx Endpoints\n"); > + return false; > + } > + > + if ((out->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 2) { > + dev_err(&intf->dev, "Invalid Tx Endpoints\n"); > + return false; > + } > + > + if ((intr->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 3) { > + dev_err(&intf->dev, "Invalid interrupt Endpoints\n"); > + return false; > + } > + > + return true; > +} We have a USB core function that does all of the above for you, why not use that instead? Look at usb_find_common_endpoints() and usb_find_common_endpoints_reverse() and at the very least usb_find_bulk_in_endpoint() and related functions. Please don't open-code this type of logic, it's easy to get things wrong. thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> > Sent: Friday, May 21, 2021 5:43 PM [...] > We have a USB core function that does all of the above for you, why not > use that instead? > > Look at usb_find_common_endpoints() and > usb_find_common_endpoints_reverse() and at the very least > usb_find_bulk_in_endpoint() and related functions. Please don't > open-code this type of logic, it's easy to get things wrong. Fine. Thanks. Best Regards, Hayes
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index bc6b2f8aaa7e..1fb85c79bd33 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -324,6 +324,7 @@ #define MAC_CLK_SPDWN_EN BIT(15) /* PLA_MAC_PWR_CTRL3 */ +#define PLA_MCU_SPDWN_EN BIT(14) #define PKT_AVAIL_SPDWN_EN 0x0100 #define SUSPEND_SPDWN_EN 0x0004 #define U1U2_SPDWN_EN 0x0002 @@ -5005,6 +5006,8 @@ static void rtl8153_down(struct r8152 *tp) static void rtl8153b_up(struct r8152 *tp) { + u32 ocp_data; + if (test_bit(RTL8152_UNPLUG, &tp->flags)) return; @@ -5015,17 +5018,27 @@ static void rtl8153b_up(struct r8152 *tp) r8153_first_init(tp); ocp_write_dword(tp, MCU_TYPE_USB, USB_RX_BUF_TH, RX_THR_B); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3); + ocp_data &= ~PLA_MCU_SPDWN_EN; + ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data); + r8153_aldps_en(tp, true); r8153b_u1u2en(tp, true); } static void rtl8153b_down(struct r8152 *tp) { + u32 ocp_data; + if (test_bit(RTL8152_UNPLUG, &tp->flags)) { rtl_drop_queued_tx(tp); return; } + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3); + ocp_data |= PLA_MCU_SPDWN_EN; + ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data); + r8153b_u1u2en(tp, false); r8153_u2p3en(tp, false); r8153b_power_cut_en(tp, false); @@ -5521,6 +5534,10 @@ static void r8153b_init(struct r8152 *tp) ocp_data |= MAC_CLK_SPDWN_EN; ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, ocp_data); + ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3); + ocp_data &= ~PLA_MCU_SPDWN_EN; + ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data); + set_bit(GREEN_ETHERNET, &tp->flags); /* rx aggregation */
PLA MCU clock speed down could only be enabled when tx/rx are disabled. Otherwise, the packet loss may occur. Signed-off-by: Hayes Wang <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)