Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Krzysztof Mazur <hidden>
Date: 2012-11-29 15:09:49
Also in:
lkml
On Wed, Nov 28, 2012 at 10:18:35PM +0000, David Woodhouse wrote:
quoted hunk ↗ jump to hunk
On Wed, 2012-11-28 at 09:21 +0000, David Laight wrote:quoted
Even when it might make sense to sleep in close until tx drains there needs to be a finite timeout before it become abortive.You are, of course, right. We should never wait for hardware for ever. And just to serve me right, I seem to have hit a bug in the latest Solos firmware (1.11) which makes it sometimes lock up when I reboot. So it never responds to the PKT_PCLOSE packet... and thus it deadlocks when I try to kill pppd and unload the module to reset it :) New version... From 53dd01c08fec5b26006a009b25e4210127fdb27a Mon Sep 17 00:00:00 2001 From: David Woodhouse <redacted> Date: Tue, 27 Nov 2012 23:49:24 +0000 Subject: [PATCH] solos-pci: Wait for pending TX to complete when releasing vcc We should no longer be calling the old pop routine for the vcc, after vcc_release() has completed. Make sure we wait for any pending TX skbs to complete, by waiting for our own PKT_PCLOSE control skb to be sent. Signed-off-by: David Woodhouse <redacted> --- drivers/atm/solos-pci.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c index 9851093..3720670 100644 --- a/drivers/atm/solos-pci.c +++ b/drivers/atm/solos-pci.c@@ -92,6 +92,7 @@ struct pkt_hdr { }; struct solos_skb_cb { + struct completion c; struct atm_vcc *vcc; uint32_t dma_addr; };@@ -881,11 +882,18 @@ static void pclose(struct atm_vcc *vcc) header->vci = cpu_to_le16(vcc->vci); header->type = cpu_to_le16(PKT_PCLOSE); + init_completion(&SKB_CB(skb)->c); + fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, NULL); clear_bit(ATM_VF_ADDR, &vcc->flags); clear_bit(ATM_VF_READY, &vcc->flags); + if (!wait_for_completion_timeout(&SKB_CB(skb)->c, + jiffies + msecs_to_jiffies(5000))) + dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n", + SOLOS_CHAN(vcc->dev)); +
I don't like two thinks about this patch: - if allos_skb(sizeof(*header), GFP_ATOMIC) at beginning of pclose() fails we will crash - if card wakes up after this timeout we will probably crash too That's why proposed different approach, but it has other problems. Krzysiek