[Linux-ATM-General] [PATCH RFC] solos-pci: Fix BUG() with shared skb

[David W. <dw...@in...>]

Simon reported this BUG():

 kernel BUG at net/core/skbuff.c:1065!
 Call Trace:
  [<f9b7c12c>] ? pppoatm_send+0x3f/0x1a0 [pppoatm]
  [<f8751797>] psend+0xa9/0x14a [solos_pci]
  [<f9b7c248>] pppoatm_send+0x15b/0x1a0 [pppoatm]
  [<f8a2f77d>] ppp_push+0x76/0x533 [ppp_generic]

(Rest of backtrace at http://s85.org/mn0aOxMN — the skb appears to be
IPv6, forwarded from another interface over PPPoATM.)

I wasn't expecting to see shared skbs in the ATM driver's ->send()
function. Is this the right fix?

I've included the whole function in the patch context... I appear to be
jumping through a lot of hand-coded hoops here, just to ensure that I
can safely prepend my own device's hardware header to the skb. This
makes me think that I'm doing something severely wrong. There ought to
be an easier way of doing this, surely? What am I missing?

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 32784d1..b15e475 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -1135,40 +1135,50 @@ static uint32_t fpga_tx(struct solos_card *card)
 static int psend(struct atm_vcc *vcc, struct sk_buff *skb)
 {
 	struct solos_card *card = vcc->dev->dev_data;
 	struct pkt_hdr *header;
 	int pktlen;
 
 	pktlen = skb->len;
 	if (pktlen > (BUF_SIZE - sizeof(*header))) {
 		dev_warn(&card->dev->dev, "Length of PDU is too large. Dropping PDU.\n");
 		solos_pop(vcc, skb);
 		return 0;
 	}
 
+	/* This is skb_share_check() except it uses solos_pop() instead of kfree_skb() */
+	if (skb_shared(skb)) {
+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
+		if (unlikely(!nskb)) {
+			solos_pop(vcc, skb);
+			return 0;
+		}
+		consume_skb(skb);
+		skb = nskb;
+	}
 	if (!skb_clone_writable(skb, sizeof(*header))) {
 		int expand_by = 0;
 		int ret;
 
 		if (skb_headroom(skb) < sizeof(*header))
 			expand_by = sizeof(*header) - skb_headroom(skb);
 
 		ret = pskb_expand_head(skb, expand_by, 0, GFP_ATOMIC);
 		if (ret) {
 			dev_warn(&card->dev->dev, "pskb_expand_head failed.\n");
 			solos_pop(vcc, skb);
 			return ret;
 		}
 	}
 
 	header = (void *)skb_push(skb, sizeof(*header));
 
 	/* This does _not_ include the size of the header */
 	header->size = cpu_to_le16(pktlen);
 	header->vpi = cpu_to_le16(vcc->vpi);
 	header->vci = cpu_to_le16(vcc->vci);
 	header->type = cpu_to_le16(PKT_DATA);
 
 	fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, vcc);
 
 	return 0;
 }



-- 
David Woodhouse                            Open Source Technology Centre
Dav...@in...                              Intel Corporation