llc: fix sk_buff refcounting in llc_conn_state_process()

If llc_conn_state_process() sees that llc_conn_service() put the skb on
a list, it will drop one fewer references to it.  This is wrong because
the current behavior is that llc_conn_service() never consumes a
reference to the skb.

The code also makes the number of skb references being dropped
conditional on which of ind_prim and cfm_prim are nonzero, yet neither
of these affects how many references are *acquired*.  So there is extra
code that tries to fix this up by sometimes taking another reference.

Remove the unnecessary/broken refcounting logic and instead just add an
skb_get() before the only two places where an extra reference is
actually consumed.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 0b0c6f1..a79b739 100644 (file)
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -64,12 +64,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
        struct llc_sock *llc = llc_sk(skb->sk);
        struct llc_conn_state_ev *ev = llc_conn_ev(skb);
 
-       /*
-        * We have to hold the skb, because llc_conn_service will kfree it in
-        * the sending path and we need to look at the skb->cb, where we encode
-        * llc_conn_state_ev.
-        */
-       skb_get(skb);
        ev->ind_prim = ev->cfm_prim = 0;
        /*
         * Send event to state machine
@@ -77,21 +71,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
        rc = llc_conn_service(skb->sk, skb);
        if (unlikely(rc != 0)) {
                printk(KERN_ERR "%s: llc_conn_service failed\n", __func__);
-               goto out_kfree_skb;
-       }
-
-       if (unlikely(!ev->ind_prim && !ev->cfm_prim)) {
-               /* indicate or confirm not required */
-               if (!skb->next)
-                       goto out_kfree_skb;
                goto out_skb_put;
        }
 
-       if (unlikely(ev->ind_prim && ev->cfm_prim)) /* Paranoia */
-               skb_get(skb);
-
        switch (ev->ind_prim) {
        case LLC_DATA_PRIM:
+               skb_get(skb);
                llc_save_primitive(sk, skb, LLC_DATA_PRIM);
                if (unlikely(sock_queue_rcv_skb(sk, skb))) {
                        /*
@@ -108,6 +93,7 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
                 * skb->sk pointing to the newly created struct sock in
                 * llc_conn_handler. -acme
                 */
+               skb_get(skb);
                skb_queue_tail(&sk->sk_receive_queue, skb);
                sk->sk_state_change(sk);
                break;
@@ -123,7 +109,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
                                sk->sk_state_change(sk);
                        }
                }
-               kfree_skb(skb);
                sock_put(sk);
                break;
        case LLC_RESET_PRIM:
@@ -132,14 +117,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
                 * RESET is not being notified to upper layers for now
                 */
                printk(KERN_INFO "%s: received a reset ind!\n", __func__);
-               kfree_skb(skb);
                break;
        default:
-               if (ev->ind_prim) {
+               if (ev->ind_prim)
                        printk(KERN_INFO "%s: received unknown %d prim!\n",
                                __func__, ev->ind_prim);
-                       kfree_skb(skb);
-               }
                /* No indication */
                break;
        }
@@ -181,15 +163,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
                printk(KERN_INFO "%s: received a reset conf!\n", __func__);
                break;
        default:
-               if (ev->cfm_prim) {
+               if (ev->cfm_prim)
                        printk(KERN_INFO "%s: received unknown %d prim!\n",
                                        __func__, ev->cfm_prim);
-                       break;
-               }
-               goto out_skb_put; /* No confirmation */
+               /* No confirmation */
+               break;
        }
-out_kfree_skb:
-       kfree_skb(skb);
 out_skb_put:
        kfree_skb(skb);
        return rc;