net: lan78xx: replace bogus endpoint lookup
authorJohan Hovold <johan@kernel.org>
Tue, 28 Jul 2020 12:10:31 +0000 (14:10 +0200)
committerDavid S. Miller <davem@davemloft.net>
Tue, 28 Jul 2020 20:35:44 +0000 (13:35 -0700)
Drop the bogus endpoint-lookup helper which could end up accepting
interfaces based on endpoints belonging to unrelated altsettings.

Note that the returned bulk pipes and interrupt endpoint descriptor
were never actually used. Instead the bulk-endpoint numbers are
hardcoded to 1 and 2 (matching the specification), while the interrupt-
endpoint descriptor was assumed to be the third descriptor created by
USB core.

Try to bring some order to this by dropping the bogus lookup helper and
adding the missing endpoint sanity checks while keeping the interrupt-
descriptor assumption for now.

Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/usb/lan78xx.c

index ee062b2..442507f 100644 (file)
@@ -377,10 +377,6 @@ struct lan78xx_net {
        struct tasklet_struct   bh;
        struct delayed_work     wq;
 
-       struct usb_host_endpoint *ep_blkin;
-       struct usb_host_endpoint *ep_blkout;
-       struct usb_host_endpoint *ep_intr;
-
        int                     msg_enable;
 
        struct urb              *urb_intr;
@@ -2860,78 +2856,12 @@ lan78xx_start_xmit(struct sk_buff *skb, struct net_device *net)
        return NETDEV_TX_OK;
 }
 
-static int
-lan78xx_get_endpoints(struct lan78xx_net *dev, struct usb_interface *intf)
-{
-       int tmp;
-       struct usb_host_interface *alt = NULL;
-       struct usb_host_endpoint *in = NULL, *out = NULL;
-       struct usb_host_endpoint *status = NULL;
-
-       for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
-               unsigned ep;
-
-               in = NULL;
-               out = NULL;
-               status = NULL;
-               alt = intf->altsetting + tmp;
-
-               for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
-                       struct usb_host_endpoint *e;
-                       int intr = 0;
-
-                       e = alt->endpoint + ep;
-                       switch (e->desc.bmAttributes) {
-                       case USB_ENDPOINT_XFER_INT:
-                               if (!usb_endpoint_dir_in(&e->desc))
-                                       continue;
-                               intr = 1;
-                               /* FALLTHROUGH */
-                       case USB_ENDPOINT_XFER_BULK:
-                               break;
-                       default:
-                               continue;
-                       }
-                       if (usb_endpoint_dir_in(&e->desc)) {
-                               if (!intr && !in)
-                                       in = e;
-                               else if (intr && !status)
-                                       status = e;
-                       } else {
-                               if (!out)
-                                       out = e;
-                       }
-               }
-               if (in && out)
-                       break;
-       }
-       if (!alt || !in || !out)
-               return -EINVAL;
-
-       dev->pipe_in = usb_rcvbulkpipe(dev->udev,
-                                      in->desc.bEndpointAddress &
-                                      USB_ENDPOINT_NUMBER_MASK);
-       dev->pipe_out = usb_sndbulkpipe(dev->udev,
-                                       out->desc.bEndpointAddress &
-                                       USB_ENDPOINT_NUMBER_MASK);
-       dev->ep_intr = status;
-
-       return 0;
-}
-
 static int lan78xx_bind(struct lan78xx_net *dev, struct usb_interface *intf)
 {
        struct lan78xx_priv *pdata = NULL;
        int ret;
        int i;
 
-       ret = lan78xx_get_endpoints(dev, intf);
-       if (ret) {
-               netdev_warn(dev->net, "lan78xx_get_endpoints failed: %d\n",
-                           ret);
-               return ret;
-       }
-
        dev->data[0] = (unsigned long)kzalloc(sizeof(*pdata), GFP_KERNEL);
 
        pdata = (struct lan78xx_priv *)(dev->data[0]);
@@ -3700,6 +3630,7 @@ static void lan78xx_stat_monitor(struct timer_list *t)
 static int lan78xx_probe(struct usb_interface *intf,
                         const struct usb_device_id *id)
 {
+       struct usb_host_endpoint *ep_blkin, *ep_blkout, *ep_intr;
        struct lan78xx_net *dev;
        struct net_device *netdev;
        struct usb_device *udev;
@@ -3748,6 +3679,34 @@ static int lan78xx_probe(struct usb_interface *intf,
 
        mutex_init(&dev->stats.access_lock);
 
+       if (intf->cur_altsetting->desc.bNumEndpoints < 3) {
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       dev->pipe_in = usb_rcvbulkpipe(udev, BULK_IN_PIPE);
+       ep_blkin = usb_pipe_endpoint(udev, dev->pipe_in);
+       if (!ep_blkin || !usb_endpoint_is_bulk_in(&ep_blkin->desc)) {
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       dev->pipe_out = usb_sndbulkpipe(udev, BULK_OUT_PIPE);
+       ep_blkout = usb_pipe_endpoint(udev, dev->pipe_out);
+       if (!ep_blkout || !usb_endpoint_is_bulk_out(&ep_blkout->desc)) {
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       ep_intr = &intf->cur_altsetting->endpoint[2];
+       if (!usb_endpoint_is_int_in(&ep_intr->desc)) {
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       dev->pipe_intr = usb_rcvintpipe(dev->udev,
+                                       usb_endpoint_num(&ep_intr->desc));
+
        ret = lan78xx_bind(dev, intf);
        if (ret < 0)
                goto out2;
@@ -3759,23 +3718,7 @@ static int lan78xx_probe(struct usb_interface *intf,
        netdev->max_mtu = MAX_SINGLE_PACKET_SIZE;
        netif_set_gso_max_size(netdev, MAX_SINGLE_PACKET_SIZE - MAX_HEADER);
 
-       if (intf->cur_altsetting->desc.bNumEndpoints < 3) {
-               ret = -ENODEV;
-               goto out3;
-       }
-
-       dev->ep_blkin = (intf->cur_altsetting)->endpoint + 0;
-       dev->ep_blkout = (intf->cur_altsetting)->endpoint + 1;
-       dev->ep_intr = (intf->cur_altsetting)->endpoint + 2;
-
-       dev->pipe_in = usb_rcvbulkpipe(udev, BULK_IN_PIPE);
-       dev->pipe_out = usb_sndbulkpipe(udev, BULK_OUT_PIPE);
-
-       dev->pipe_intr = usb_rcvintpipe(dev->udev,
-                                       dev->ep_intr->desc.bEndpointAddress &
-                                       USB_ENDPOINT_NUMBER_MASK);
-       period = dev->ep_intr->desc.bInterval;
-
+       period = ep_intr->desc.bInterval;
        maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
        buf = kmalloc(maxp, GFP_KERNEL);
        if (buf) {