NFC: llcp: Fix Rx memory leak
authorSamuel Ortiz <sameo@linux.intel.com>
Fri, 7 Dec 2012 15:37:30 +0000 (16:37 +0100)
committerSamuel Ortiz <sameo@linux.intel.com>
Wed, 9 Jan 2013 23:48:25 +0000 (00:48 +0100)
The reference count bump on the llcp Rx path is leading to a memory leak
whenever we're not receiving an I frame.
We fix that by removing the refcount bump (drivers must not free their
received skb) and using it only in the I frame path, when the frame is
actually queued. In that case, the skb will only be freed when someone
fetches it from userspace. in all other cases, LLCP received frames will
be freed when leaving the Rx work queue.

Reported-by: Eric Lapuyade <eric.lapuyade@linux.intel.com>
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
net/nfc/llcp/llcp.c

index 003c82fe8bd7d9b16398e5b4cc810ee215d349bf..85bc75c38dea67544a39d7e352b144f3a8f4af0e 100644 (file)
@@ -782,8 +782,14 @@ static void nfc_llcp_recv_ui(struct nfc_llcp_local *local,
 
        /* There is no sequence with UI frames */
        skb_pull(skb, LLCP_HEADER_SIZE);
-       if (sock_queue_rcv_skb(&llcp_sock->sk, skb)) {
-               pr_err("receive queue is full\n");
+       if (!sock_queue_rcv_skb(&llcp_sock->sk, skb)) {
+               /*
+                * UI frames will be freed from the socket layer, so we
+                * need to keep them alive until someone receives them.
+                */
+               skb_get(skb);
+       } else {
+               pr_err("Receive queue is full\n");
                kfree_skb(skb);
        }
 
@@ -977,8 +983,14 @@ static void nfc_llcp_recv_hdlc(struct nfc_llcp_local *local,
                        pr_err("Received out of sequence I PDU\n");
 
                skb_pull(skb, LLCP_HEADER_SIZE + LLCP_SEQUENCE_SIZE);
-               if (sock_queue_rcv_skb(&llcp_sock->sk, skb)) {
-                       pr_err("receive queue is full\n");
+               if (!sock_queue_rcv_skb(&llcp_sock->sk, skb)) {
+                       /*
+                        * I frames will be freed from the socket layer, so we
+                        * need to keep them alive until someone receives them.
+                        */
+                       skb_get(skb);
+               } else {
+                       pr_err("Receive queue is full\n");
                        kfree_skb(skb);
                }
        }
@@ -1299,6 +1311,13 @@ static void nfc_llcp_rx_work(struct work_struct *work)
        local->rx_pending = NULL;
 }
 
+static void __nfc_llcp_recv(struct nfc_llcp_local *local, struct sk_buff *skb)
+{
+       local->rx_pending = skb;
+       del_timer(&local->link_timer);
+       schedule_work(&local->rx_work);
+}
+
 void nfc_llcp_recv(void *data, struct sk_buff *skb, int err)
 {
        struct nfc_llcp_local *local = (struct nfc_llcp_local *) data;
@@ -1309,9 +1328,7 @@ void nfc_llcp_recv(void *data, struct sk_buff *skb, int err)
                return;
        }
 
-       local->rx_pending = skb_get(skb);
-       del_timer(&local->link_timer);
-       schedule_work(&local->rx_work);
+       __nfc_llcp_recv(local, skb);
 }
 
 int nfc_llcp_data_received(struct nfc_dev *dev, struct sk_buff *skb)
@@ -1322,9 +1339,7 @@ int nfc_llcp_data_received(struct nfc_dev *dev, struct sk_buff *skb)
        if (local == NULL)
                return -ENODEV;
 
-       local->rx_pending = skb_get(skb);
-       del_timer(&local->link_timer);
-       schedule_work(&local->rx_work);
+       __nfc_llcp_recv(local, skb);
 
        return 0;
 }