]> git.kernelconcepts.de Git - karo-tx-linux.git/commitdiff
Merge git://git.infradead.org/users/dwmw2/atm
authorDavid S. Miller <davem@davemloft.net>
Sun, 2 Dec 2012 01:45:24 +0000 (20:45 -0500)
committerDavid S. Miller <davem@davemloft.net>
Sun, 2 Dec 2012 01:45:24 +0000 (20:45 -0500)
David Woodhouse says:

====================
This is the result of pulling on the thread started by Krzysztof Mazur's
original patch 'pppoatm: don't send frames to destroyed vcc'.

Various problems in the pppoatm and br2684 code are solved, some of which
were easily triggered and would panic the kernel.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/atm/solos-pci.c
include/linux/atmdev.h
net/atm/br2684.c
net/atm/common.c
net/atm/pppoatm.c

index 98510931c8153110ee0617457fb63cafe652696a..6619a8a9607ce7be38bfe3cbc08e69215cfd4345 100644 (file)
@@ -164,7 +164,6 @@ static void fpga_queue(struct solos_card *card, int port, struct sk_buff *skb,
 static uint32_t fpga_tx(struct solos_card *);
 static irqreturn_t solos_irq(int irq, void *dev_id);
 static struct atm_vcc* find_vcc(struct atm_dev *dev, short vpi, int vci);
-static int list_vccs(int vci);
 static int atm_init(struct solos_card *, struct device *);
 static void atm_remove(struct solos_card *);
 static int send_command(struct solos_card *card, int dev, const char *buf, size_t size);
@@ -710,7 +709,8 @@ void solos_bh(unsigned long card_arg)
                                                dev_warn(&card->dev->dev, "Received packet for unknown VPI.VCI %d.%d on port %d\n",
                                                         le16_to_cpu(header->vpi), le16_to_cpu(header->vci),
                                                         port);
-                                       continue;
+                                       dev_kfree_skb_any(skb);
+                                       break;
                                }
                                atm_charge(vcc, skb->truesize);
                                vcc->push(vcc, skb);
@@ -790,44 +790,6 @@ static struct atm_vcc *find_vcc(struct atm_dev *dev, short vpi, int vci)
        return vcc;
 }
 
-static int list_vccs(int vci)
-{
-       struct hlist_head *head;
-       struct atm_vcc *vcc;
-       struct hlist_node *node;
-       struct sock *s;
-       int num_found = 0;
-       int i;
-
-       read_lock(&vcc_sklist_lock);
-       if (vci != 0){
-               head = &vcc_hash[vci & (VCC_HTABLE_SIZE -1)];
-               sk_for_each(s, node, head) {
-                       num_found ++;
-                       vcc = atm_sk(s);
-                       printk(KERN_DEBUG "Device: %d Vpi: %d Vci: %d\n",
-                              vcc->dev->number,
-                              vcc->vpi,
-                              vcc->vci);
-               }
-       } else {
-               for(i = 0; i < VCC_HTABLE_SIZE; i++){
-                       head = &vcc_hash[i];
-                       sk_for_each(s, node, head) {
-                               num_found ++;
-                               vcc = atm_sk(s);
-                               printk(KERN_DEBUG "Device: %d Vpi: %d Vci: %d\n",
-                                      vcc->dev->number,
-                                      vcc->vpi,
-                                      vcc->vci);
-                       }
-               }
-       }
-       read_unlock(&vcc_sklist_lock);
-       return num_found;
-}
-
-
 static int popen(struct atm_vcc *vcc)
 {
        struct solos_card *card = vcc->dev->dev_data;
@@ -840,7 +802,7 @@ static int popen(struct atm_vcc *vcc)
                return -EINVAL;
        }
 
-       skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+       skb = alloc_skb(sizeof(*header), GFP_KERNEL);
        if (!skb) {
                if (net_ratelimit())
                        dev_warn(&card->dev->dev, "Failed to allocate sk_buff in popen()\n");
@@ -857,8 +819,6 @@ static int popen(struct atm_vcc *vcc)
 
        set_bit(ATM_VF_ADDR, &vcc->flags);
        set_bit(ATM_VF_READY, &vcc->flags);
-       list_vccs(0);
-
 
        return 0;
 }
@@ -866,10 +826,21 @@ static int popen(struct atm_vcc *vcc)
 static void pclose(struct atm_vcc *vcc)
 {
        struct solos_card *card = vcc->dev->dev_data;
-       struct sk_buff *skb;
+       unsigned char port = SOLOS_CHAN(vcc->dev);
+       struct sk_buff *skb, *tmpskb;
        struct pkt_hdr *header;
 
-       skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+       /* Remove any yet-to-be-transmitted packets from the pending queue */
+       spin_lock(&card->tx_queue_lock);
+       skb_queue_walk_safe(&card->tx_queue[port], skb, tmpskb) {
+               if (SKB_CB(skb)->vcc == vcc) {
+                       skb_unlink(skb, &card->tx_queue[port]);
+                       solos_pop(vcc, skb);
+               }
+       }
+       spin_unlock(&card->tx_queue_lock);
+
+       skb = alloc_skb(sizeof(*header), GFP_KERNEL);
        if (!skb) {
                dev_warn(&card->dev->dev, "Failed to allocate sk_buff in pclose()\n");
                return;
@@ -881,15 +852,22 @@ static void pclose(struct atm_vcc *vcc)
        header->vci = cpu_to_le16(vcc->vci);
        header->type = cpu_to_le16(PKT_PCLOSE);
 
-       fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, NULL);
+       skb_get(skb);
+       fpga_queue(card, port, skb, NULL);
 
-       clear_bit(ATM_VF_ADDR, &vcc->flags);
-       clear_bit(ATM_VF_READY, &vcc->flags);
+       if (!wait_event_timeout(card->param_wq, !skb_shared(skb), 5 * HZ))
+               dev_warn(&card->dev->dev,
+                        "Timeout waiting for VCC close on port %d\n", port);
+
+       dev_kfree_skb(skb);
 
        /* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the
           tasklet has finished processing any incoming packets (and, more to
           the point, using the vcc pointer). */
        tasklet_unlock_wait(&card->tlet);
+
+       clear_bit(ATM_VF_ADDR, &vcc->flags);
+
        return;
 }
 
@@ -1011,9 +989,10 @@ static uint32_t fpga_tx(struct solos_card *card)
                        if (vcc) {
                                atomic_inc(&vcc->stats->tx);
                                solos_pop(vcc, oldskb);
-                       } else
+                       } else {
                                dev_kfree_skb_irq(oldskb);
-
+                               wake_up(&card->param_wq);
+                       }
                }
        }
        /* For non-DMA TX, write the 'TX start' bit for all four ports simultaneously */
@@ -1248,7 +1227,7 @@ static int atm_init(struct solos_card *card, struct device *parent)
                card->atmdev[i]->phy_data = (void *)(unsigned long)i;
                atm_dev_signal_change(card->atmdev[i], ATM_PHY_SIG_FOUND);
 
-               skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+               skb = alloc_skb(sizeof(*header), GFP_KERNEL);
                if (!skb) {
                        dev_warn(&card->dev->dev, "Failed to allocate sk_buff in atm_init()\n");
                        continue;
@@ -1345,6 +1324,8 @@ static struct pci_driver fpga_driver = {
 
 static int __init solos_pci_init(void)
 {
+       BUILD_BUG_ON(sizeof(struct solos_skb_cb) > sizeof(((struct sk_buff *)0)->cb));
+
        printk(KERN_INFO "Solos PCI Driver Version %s\n", VERSION);
        return pci_register_driver(&fpga_driver);
 }
index 22ef21c33d0c600f5adf48d34e5bdde4687d76c4..c1da539f5e28a965702c39d21375923e8271f67f 100644 (file)
@@ -99,6 +99,7 @@ struct atm_vcc {
        struct atm_dev  *dev;           /* device back pointer */
        struct atm_qos  qos;            /* QOS */
        struct atm_sap  sap;            /* SAP */
+       void (*release_cb)(struct atm_vcc *vcc); /* release_sock callback */
        void (*push)(struct atm_vcc *vcc,struct sk_buff *skb);
        void (*pop)(struct atm_vcc *vcc,struct sk_buff *skb); /* optional */
        int (*push_oam)(struct atm_vcc *vcc,void *cell);
@@ -106,6 +107,7 @@ struct atm_vcc {
        void            *dev_data;      /* per-device data */
        void            *proto_data;    /* per-protocol data */
        struct k_atm_aal_stats *stats;  /* pointer to AAL stats group */
+       struct module *owner;           /* owner of ->push function */
        /* SVC part --- may move later ------------------------------------- */
        short           itf;            /* interface number */
        struct sockaddr_atmsvc local;
index 8eb6fbe8d8dd681fd36e4897d98d9105bca59ea3..403e71fa88feb900a7d194f3f3c83f4f6e1cb0d4 100644 (file)
@@ -68,6 +68,8 @@ struct br2684_vcc {
        /* keep old push, pop functions for chaining */
        void (*old_push)(struct atm_vcc *vcc, struct sk_buff *skb);
        void (*old_pop)(struct atm_vcc *vcc, struct sk_buff *skb);
+       void (*old_release_cb)(struct atm_vcc *vcc);
+       struct module *old_owner;
        enum br2684_encaps encaps;
        struct list_head brvccs;
 #ifdef CONFIG_ATM_BR2684_IPFILTER
@@ -269,6 +271,17 @@ static int br2684_xmit_vcc(struct sk_buff *skb, struct net_device *dev,
        return !atmvcc->send(atmvcc, skb);
 }
 
+static void br2684_release_cb(struct atm_vcc *atmvcc)
+{
+       struct br2684_vcc *brvcc = BR2684_VCC(atmvcc);
+
+       if (atomic_read(&brvcc->qspace) > 0)
+               netif_wake_queue(brvcc->device);
+
+       if (brvcc->old_release_cb)
+               brvcc->old_release_cb(atmvcc);
+}
+
 static inline struct br2684_vcc *pick_outgoing_vcc(const struct sk_buff *skb,
                                                   const struct br2684_dev *brdev)
 {
@@ -280,6 +293,8 @@ static netdev_tx_t br2684_start_xmit(struct sk_buff *skb,
 {
        struct br2684_dev *brdev = BRPRIV(dev);
        struct br2684_vcc *brvcc;
+       struct atm_vcc *atmvcc;
+       netdev_tx_t ret = NETDEV_TX_OK;
 
        pr_debug("skb_dst(skb)=%p\n", skb_dst(skb));
        read_lock(&devs_lock);
@@ -290,9 +305,26 @@ static netdev_tx_t br2684_start_xmit(struct sk_buff *skb,
                dev->stats.tx_carrier_errors++;
                /* netif_stop_queue(dev); */
                dev_kfree_skb(skb);
-               read_unlock(&devs_lock);
-               return NETDEV_TX_OK;
+               goto out_devs;
        }
+       atmvcc = brvcc->atmvcc;
+
+       bh_lock_sock(sk_atm(atmvcc));
+
+       if (test_bit(ATM_VF_RELEASED, &atmvcc->flags) ||
+           test_bit(ATM_VF_CLOSE, &atmvcc->flags) ||
+           !test_bit(ATM_VF_READY, &atmvcc->flags)) {
+               dev->stats.tx_dropped++;
+               dev_kfree_skb(skb);
+               goto out;
+       }
+
+       if (sock_owned_by_user(sk_atm(atmvcc))) {
+               netif_stop_queue(brvcc->device);
+               ret = NETDEV_TX_BUSY;
+               goto out;
+       }
+
        if (!br2684_xmit_vcc(skb, dev, brvcc)) {
                /*
                 * We should probably use netif_*_queue() here, but that
@@ -304,8 +336,11 @@ static netdev_tx_t br2684_start_xmit(struct sk_buff *skb,
                dev->stats.tx_errors++;
                dev->stats.tx_fifo_errors++;
        }
+ out:
+       bh_unlock_sock(sk_atm(atmvcc));
+ out_devs:
        read_unlock(&devs_lock);
-       return NETDEV_TX_OK;
+       return ret;
 }
 
 /*
@@ -378,9 +413,10 @@ static void br2684_close_vcc(struct br2684_vcc *brvcc)
        list_del(&brvcc->brvccs);
        write_unlock_irq(&devs_lock);
        brvcc->atmvcc->user_back = NULL;        /* what about vcc->recvq ??? */
+       brvcc->atmvcc->release_cb = brvcc->old_release_cb;
        brvcc->old_push(brvcc->atmvcc, NULL);   /* pass on the bad news */
+       module_put(brvcc->old_owner);
        kfree(brvcc);
-       module_put(THIS_MODULE);
 }
 
 /* when AAL5 PDU comes in: */
@@ -554,9 +590,13 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
        brvcc->encaps = (enum br2684_encaps)be.encaps;
        brvcc->old_push = atmvcc->push;
        brvcc->old_pop = atmvcc->pop;
+       brvcc->old_release_cb = atmvcc->release_cb;
+       brvcc->old_owner = atmvcc->owner;
        barrier();
        atmvcc->push = br2684_push;
        atmvcc->pop = br2684_pop;
+       atmvcc->release_cb = br2684_release_cb;
+       atmvcc->owner = THIS_MODULE;
 
        /* initialize netdev carrier state */
        if (atmvcc->dev->signal == ATM_PHY_SIG_LOST)
@@ -695,10 +735,13 @@ static int br2684_ioctl(struct socket *sock, unsigned int cmd,
                        return -ENOIOCTLCMD;
                if (!capable(CAP_NET_ADMIN))
                        return -EPERM;
-               if (cmd == ATM_SETBACKEND)
+               if (cmd == ATM_SETBACKEND) {
+                       if (sock->state != SS_CONNECTED)
+                               return -EINVAL;
                        return br2684_regvcc(atmvcc, argp);
-               else
+               } else {
                        return br2684_create(argp);
+               }
 #ifdef CONFIG_ATM_BR2684_IPFILTER
        case BR2684_SETFILT:
                if (atmvcc->push != br2684_push)
index 0c0ad930a632f23e4c0661a20684c5d572e3dc15..806fc0a400514b677ca1d74abf1c55a055097822 100644 (file)
@@ -126,10 +126,19 @@ static void vcc_write_space(struct sock *sk)
        rcu_read_unlock();
 }
 
+static void vcc_release_cb(struct sock *sk)
+{
+       struct atm_vcc *vcc = atm_sk(sk);
+
+       if (vcc->release_cb)
+               vcc->release_cb(vcc);
+}
+
 static struct proto vcc_proto = {
        .name     = "VCC",
        .owner    = THIS_MODULE,
        .obj_size = sizeof(struct atm_vcc),
+       .release_cb = vcc_release_cb,
 };
 
 int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
@@ -156,7 +165,9 @@ int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
        atomic_set(&sk->sk_rmem_alloc, 0);
        vcc->push = NULL;
        vcc->pop = NULL;
+       vcc->owner = NULL;
        vcc->push_oam = NULL;
+       vcc->release_cb = NULL;
        vcc->vpi = vcc->vci = 0; /* no VCI/VPI yet */
        vcc->atm_options = vcc->aal_options = 0;
        sk->sk_destruct = vcc_sock_destruct;
@@ -175,6 +186,7 @@ static void vcc_destroy_socket(struct sock *sk)
                        vcc->dev->ops->close(vcc);
                if (vcc->push)
                        vcc->push(vcc, NULL); /* atmarpd has no push */
+               module_put(vcc->owner);
 
                while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
                        atm_return(vcc, skb->truesize);
index 226dca9894487fd7fc1b391e827acef905a44d90..8c93267ce96910a48f9eb9e5bb8e295470656edf 100644 (file)
@@ -60,6 +60,8 @@ struct pppoatm_vcc {
        struct atm_vcc  *atmvcc;        /* VCC descriptor */
        void (*old_push)(struct atm_vcc *, struct sk_buff *);
        void (*old_pop)(struct atm_vcc *, struct sk_buff *);
+       void (*old_release_cb)(struct atm_vcc *);
+       struct module *old_owner;
                                        /* keep old push/pop for detaching */
        enum pppoatm_encaps encaps;
        atomic_t inflight;
@@ -107,6 +109,24 @@ static void pppoatm_wakeup_sender(unsigned long arg)
        ppp_output_wakeup((struct ppp_channel *) arg);
 }
 
+static void pppoatm_release_cb(struct atm_vcc *atmvcc)
+{
+       struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
+
+       /*
+        * As in pppoatm_pop(), it's safe to clear the BLOCKED bit here because
+        * the wakeup *can't* race with pppoatm_send(). They both hold the PPP
+        * channel's ->downl lock. And the potential race with *setting* it,
+        * which leads to the double-check dance in pppoatm_may_send(), doesn't
+        * exist here. In the sock_owned_by_user() case in pppoatm_send(), we
+        * set the BLOCKED bit while the socket is still locked. We know that
+        * ->release_cb() can't be called until that's done.
+        */
+       if (test_and_clear_bit(BLOCKED, &pvcc->blocked))
+               tasklet_schedule(&pvcc->wakeup_tasklet);
+       if (pvcc->old_release_cb)
+               pvcc->old_release_cb(atmvcc);
+}
 /*
  * This gets called every time the ATM card has finished sending our
  * skb.  The ->old_pop will take care up normal atm flow control,
@@ -151,12 +171,11 @@ static void pppoatm_unassign_vcc(struct atm_vcc *atmvcc)
        pvcc = atmvcc_to_pvcc(atmvcc);
        atmvcc->push = pvcc->old_push;
        atmvcc->pop = pvcc->old_pop;
+       atmvcc->release_cb = pvcc->old_release_cb;
        tasklet_kill(&pvcc->wakeup_tasklet);
        ppp_unregister_channel(&pvcc->chan);
        atmvcc->user_back = NULL;
        kfree(pvcc);
-       /* Gee, I hope we have the big kernel lock here... */
-       module_put(THIS_MODULE);
 }
 
 /* Called when an AAL5 PDU comes in */
@@ -165,9 +184,13 @@ static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
        struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
        pr_debug("\n");
        if (skb == NULL) {                      /* VCC was closed */
+               struct module *module;
+
                pr_debug("removing ATMPPP VCC %p\n", pvcc);
+               module = pvcc->old_owner;
                pppoatm_unassign_vcc(atmvcc);
                atmvcc->push(atmvcc, NULL);     /* Pass along bad news */
+               module_put(module);
                return;
        }
        atm_return(atmvcc, skb->truesize);
@@ -211,7 +234,7 @@ error:
        ppp_input_error(&pvcc->chan, 0);
 }
 
-static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
+static int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
 {
        /*
         * It's not clear that we need to bother with using atm_may_send()
@@ -269,10 +292,33 @@ static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
 static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
 {
        struct pppoatm_vcc *pvcc = chan_to_pvcc(chan);
+       struct atm_vcc *vcc;
+       int ret;
+
        ATM_SKB(skb)->vcc = pvcc->atmvcc;
        pr_debug("(skb=0x%p, vcc=0x%p)\n", skb, pvcc->atmvcc);
        if (skb->data[0] == '\0' && (pvcc->flags & SC_COMP_PROT))
                (void) skb_pull(skb, 1);
+
+       vcc = ATM_SKB(skb)->vcc;
+       bh_lock_sock(sk_atm(vcc));
+       if (sock_owned_by_user(sk_atm(vcc))) {
+               /*
+                * Needs to happen (and be flushed, hence test_and_) before we unlock
+                * the socket. It needs to be seen by the time our ->release_cb gets
+                * called.
+                */
+               test_and_set_bit(BLOCKED, &pvcc->blocked);
+               goto nospace;
+       }
+       if (test_bit(ATM_VF_RELEASED, &vcc->flags) ||
+           test_bit(ATM_VF_CLOSE, &vcc->flags) ||
+           !test_bit(ATM_VF_READY, &vcc->flags)) {
+               bh_unlock_sock(sk_atm(vcc));
+               kfree_skb(skb);
+               return DROP_PACKET;
+       }
+
        switch (pvcc->encaps) {         /* LLC encapsulation needed */
        case e_llc:
                if (skb_headroom(skb) < LLC_LEN) {
@@ -285,8 +331,10 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
                        }
                        consume_skb(skb);
                        skb = n;
-                       if (skb == NULL)
+                       if (skb == NULL) {
+                               bh_unlock_sock(sk_atm(vcc));
                                return DROP_PACKET;
+                       }
                } else if (!pppoatm_may_send(pvcc, skb->truesize))
                        goto nospace;
                memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
@@ -296,6 +344,7 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
                        goto nospace;
                break;
        case e_autodetect:
+               bh_unlock_sock(sk_atm(vcc));
                pr_debug("Trying to send without setting encaps!\n");
                kfree_skb(skb);
                return 1;
@@ -305,9 +354,12 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
        ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
        pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n",
                 skb, ATM_SKB(skb)->vcc, ATM_SKB(skb)->vcc->dev);
-       return ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
+       ret = ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
            ? DROP_PACKET : 1;
+       bh_unlock_sock(sk_atm(vcc));
+       return ret;
 nospace:
+       bh_unlock_sock(sk_atm(vcc));
        /*
         * We don't have space to send this SKB now, but we might have
         * already applied SC_COMP_PROT compression, so may need to undo
@@ -362,6 +414,8 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
        atomic_set(&pvcc->inflight, NONE_INFLIGHT);
        pvcc->old_push = atmvcc->push;
        pvcc->old_pop = atmvcc->pop;
+       pvcc->old_owner = atmvcc->owner;
+       pvcc->old_release_cb = atmvcc->release_cb;
        pvcc->encaps = (enum pppoatm_encaps) be.encaps;
        pvcc->chan.private = pvcc;
        pvcc->chan.ops = &pppoatm_ops;
@@ -377,7 +431,9 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
        atmvcc->user_back = pvcc;
        atmvcc->push = pppoatm_push;
        atmvcc->pop = pppoatm_pop;
+       atmvcc->release_cb = pppoatm_release_cb;
        __module_get(THIS_MODULE);
+       atmvcc->owner = THIS_MODULE;
 
        /* re-process everything received between connection setup and
           backend setup */
@@ -406,6 +462,8 @@ static int pppoatm_ioctl(struct socket *sock, unsigned int cmd,
                        return -ENOIOCTLCMD;
                if (!capable(CAP_NET_ADMIN))
                        return -EPERM;
+               if (sock->state != SS_CONNECTED)
+                       return -EINVAL;
                return pppoatm_assign_vcc(atmvcc, argp);
                }
        case PPPIOCGCHAN: