netdev - Re: [PATCH 00/17] ATM fixes for pppoatm/br2684

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 02 Dec 2012 00:35:47 +0000
From:	David Woodhouse <dwmw2@...radead.org>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, chas@....nrl.navy.mil,
	krzysiek@...lesie.net
Subject: Re: [PATCH 00/17] ATM fixes for pppoatm/br2684

On Sat, 2012-12-01 at 11:44 -0500, David Miller wrote:
> 
> > drivers/atm/solos-pci.c: In function ‘solos_pci_init’:
> > drivers/atm/solos-pci.c:1329:2: error: size of unnamed array is
> negative
> 
> It's from adding the completion to the solos skb cb, you can't do
> that.  It won't fit on 64-bit when all debugging kconfig options are
> enabled.

Thanks for catching that. I've just posted a [v2] version of the
offending patch, which no longer puts a completion into the skb cb.

I'd appreciate a slightly more clueful eye looking over the incremental
patch (below) just to confirm that the new method is correct, but it
certainly seems to work. This version is identical to the one I posted
earlier, except that I use dev_kfree_skb() in the pclose() function
instead of dev_kfree_skb_any(). We know this will be called from a
suitable context, and it even uses GFP_KERNEL a few lines higher up.

If that's OK, please pull the resulting tree from
	git://git.infradead.org/users/dwmw2/atm.git


diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index e59bcfd..6619a8a 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -92,7 +92,6 @@ struct pkt_hdr {
 };
 
 struct solos_skb_cb {
-	struct completion c;
 	struct atm_vcc *vcc;
 	uint32_t dma_addr;
 };
@@ -853,13 +852,14 @@ 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);
-
+	skb_get(skb);
 	fpga_queue(card, port, skb, NULL);
 
-	if (!wait_for_completion_timeout(&SKB_CB(skb)->c, 5 * HZ))
-		dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n",
-			 port);
+	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
@@ -990,10 +990,8 @@ static uint32_t fpga_tx(struct solos_card *card)
 				atomic_inc(&vcc->stats->tx);
 				solos_pop(vcc, oldskb);
 			} else {
-				struct pkt_hdr *header = (void *)oldskb->data;
-				if (le16_to_cpu(header->type) == PKT_PCLOSE)
-					complete(&SKB_CB(oldskb)->c);
 				dev_kfree_skb_irq(oldskb);
+				wake_up(&card->param_wq);
 			}
 		}
 	}

-- 
dwmw2


Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (6171 bytes)