NFC: digital: Fix handling of saved PDU sk_buff pointers
authorThierry Escande <thierry.escande@collabora.com>
Fri, 8 Jul 2016 13:52:39 +0000 (15:52 +0200)
committerSamuel Ortiz <sameo@linux.intel.com>
Sun, 10 Jul 2016 23:55:42 +0000 (01:55 +0200)
This patch fixes the way an I-PDU is saved in case it needs to be sent
again. It is now copied using pskb_copy() and not simply referenced
using skb_get() since it could be modified by the driver.

digital_in_send_saved_skb() and digital_tg_send_saved_skb() still get a
reference on the saved skb which is re-sent but release it if the send
operation fails. That way the caller doesn't have to take care about skb
ref in case of error.

RTOX supervisor PDU must not be saved as this can override a previously
saved I-PDU that should be re-sent later on.

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
include/net/nfc/digital.h
net/nfc/digital_dep.c

index 506e3f6eabefc1793630f4193c22d27b44b6168b..f9a4e4771861faec25e1baff0bc1d7105fa1c109 100644 (file)
@@ -237,7 +237,6 @@ struct nfc_digital_dev {
        int nack_count;
 
        struct sk_buff *saved_skb;
-       unsigned int saved_skb_len;
 
        u16 target_fsc;
 
index b62c85dc12a2e5b6666c69b75a7c6272a6a4573f..804585cb3f8eafe57585c7032bf8619f1ebed8ed 100644 (file)
@@ -524,8 +524,7 @@ static int digital_in_send_ack(struct nfc_digital_dev *ddev,
 
        ddev->skb_add_crc(skb);
 
-       ddev->saved_skb = skb_get(skb);
-       ddev->saved_skb_len = skb->len;
+       ddev->saved_skb = pskb_copy(skb, GFP_KERNEL);
 
        rc = digital_in_send_cmd(ddev, skb, 1500, digital_in_recv_dep_res,
                                 data_exch);
@@ -627,16 +626,10 @@ static int digital_in_send_rtox(struct nfc_digital_dev *ddev,
 
        ddev->skb_add_crc(skb);
 
-       ddev->saved_skb = skb_get(skb);
-       ddev->saved_skb_len = skb->len;
-
        rc = digital_in_send_cmd(ddev, skb, 1500, digital_in_recv_dep_res,
                                 data_exch);
-       if (rc) {
+       if (rc)
                kfree_skb(skb);
-               kfree_skb(ddev->saved_skb);
-               ddev->saved_skb = NULL;
-       }
 
        return rc;
 }
@@ -644,11 +637,19 @@ static int digital_in_send_rtox(struct nfc_digital_dev *ddev,
 static int digital_in_send_saved_skb(struct nfc_digital_dev *ddev,
                                     struct digital_data_exch *data_exch)
 {
+       int rc;
+
+       if (!ddev->saved_skb)
+               return -EINVAL;
+
        skb_get(ddev->saved_skb);
-       skb_push(ddev->saved_skb, ddev->saved_skb_len);
 
-       return digital_in_send_cmd(ddev, ddev->saved_skb, 1500,
-                                  digital_in_recv_dep_res, data_exch);
+       rc = digital_in_send_cmd(ddev, ddev->saved_skb, 1500,
+                                digital_in_recv_dep_res, data_exch);
+       if (rc)
+               kfree_skb(ddev->saved_skb);
+
+       return rc;
 }
 
 static void digital_in_recv_dep_res(struct nfc_digital_dev *ddev, void *arg,
@@ -812,17 +813,12 @@ static void digital_in_recv_dep_res(struct nfc_digital_dev *ddev, void *arg,
        case DIGITAL_NFC_DEP_PFB_SUPERVISOR_PDU:
                if (!DIGITAL_NFC_DEP_PFB_IS_TIMEOUT(pfb)) { /* ATN */
                        rc = digital_in_send_saved_skb(ddev, data_exch);
-                       if (rc) {
-                               kfree_skb(ddev->saved_skb);
+                       if (rc)
                                goto error;
-                       }
 
                        return;
                }
 
-               kfree_skb(ddev->saved_skb);
-               ddev->saved_skb = NULL;
-
                rc = digital_in_send_rtox(ddev, data_exch, resp->data[0]);
                if (rc)
                        goto error;
@@ -876,8 +872,7 @@ int digital_in_send_dep_req(struct nfc_digital_dev *ddev,
 
        ddev->skb_add_crc(tmp_skb);
 
-       ddev->saved_skb = skb_get(tmp_skb);
-       ddev->saved_skb_len = tmp_skb->len;
+       ddev->saved_skb = pskb_copy(tmp_skb, GFP_KERNEL);
 
        rc = digital_in_send_cmd(ddev, tmp_skb, 1500, digital_in_recv_dep_res,
                                 data_exch);
@@ -956,8 +951,7 @@ static int digital_tg_send_ack(struct nfc_digital_dev *ddev,
 
        ddev->skb_add_crc(skb);
 
-       ddev->saved_skb = skb_get(skb);
-       ddev->saved_skb_len = skb->len;
+       ddev->saved_skb = pskb_copy(skb, GFP_KERNEL);
 
        rc = digital_tg_send_cmd(ddev, skb, 1500, digital_tg_recv_dep_req,
                                 data_exch);
@@ -1009,11 +1003,19 @@ static int digital_tg_send_atn(struct nfc_digital_dev *ddev)
 
 static int digital_tg_send_saved_skb(struct nfc_digital_dev *ddev)
 {
+       int rc;
+
+       if (!ddev->saved_skb)
+               return -EINVAL;
+
        skb_get(ddev->saved_skb);
-       skb_push(ddev->saved_skb, ddev->saved_skb_len);
 
-       return digital_tg_send_cmd(ddev, ddev->saved_skb, 1500,
-                                  digital_tg_recv_dep_req, NULL);
+       rc = digital_tg_send_cmd(ddev, ddev->saved_skb, 1500,
+                                digital_tg_recv_dep_req, NULL);
+       if (rc)
+               kfree_skb(ddev->saved_skb);
+
+       return rc;
 }
 
 static void digital_tg_recv_dep_req(struct nfc_digital_dev *ddev, void *arg,
@@ -1163,10 +1165,8 @@ static void digital_tg_recv_dep_req(struct nfc_digital_dev *ddev, void *arg,
                        ddev->atn_count = 0;
 
                        rc = digital_tg_send_saved_skb(ddev);
-                       if (rc) {
-                               kfree_skb(ddev->saved_skb);
+                       if (rc)
                                goto exit;
-                       }
                }
 
                return;
@@ -1235,8 +1235,7 @@ int digital_tg_send_dep_res(struct nfc_digital_dev *ddev, struct sk_buff *skb)
 
        ddev->skb_add_crc(tmp_skb);
 
-       ddev->saved_skb = skb_get(tmp_skb);
-       ddev->saved_skb_len = tmp_skb->len;
+       ddev->saved_skb = pskb_copy(tmp_skb, GFP_KERNEL);
 
        rc = digital_tg_send_cmd(ddev, tmp_skb, 1500, digital_tg_recv_dep_req,
                                 NULL);