Commit 36453c85 authored by Eric Biggers's avatar Eric Biggers Committed by Jakub Kicinski

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: 1da177e4 ("Linux-2.6.12-rc2")
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
parent fc8d5db1
...@@ -64,12 +64,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -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_sock *llc = llc_sk(skb->sk);
struct llc_conn_state_ev *ev = llc_conn_ev(skb); 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; ev->ind_prim = ev->cfm_prim = 0;
/* /*
* Send event to state machine * Send event to state machine
...@@ -77,21 +71,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -77,21 +71,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
rc = llc_conn_service(skb->sk, skb); rc = llc_conn_service(skb->sk, skb);
if (unlikely(rc != 0)) { if (unlikely(rc != 0)) {
printk(KERN_ERR "%s: llc_conn_service failed\n", __func__); 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; goto out_skb_put;
} }
if (unlikely(ev->ind_prim && ev->cfm_prim)) /* Paranoia */
skb_get(skb);
switch (ev->ind_prim) { switch (ev->ind_prim) {
case LLC_DATA_PRIM: case LLC_DATA_PRIM:
skb_get(skb);
llc_save_primitive(sk, skb, LLC_DATA_PRIM); llc_save_primitive(sk, skb, LLC_DATA_PRIM);
if (unlikely(sock_queue_rcv_skb(sk, skb))) { if (unlikely(sock_queue_rcv_skb(sk, skb))) {
/* /*
...@@ -108,6 +93,7 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *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 * skb->sk pointing to the newly created struct sock in
* llc_conn_handler. -acme * llc_conn_handler. -acme
*/ */
skb_get(skb);
skb_queue_tail(&sk->sk_receive_queue, skb); skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_state_change(sk); sk->sk_state_change(sk);
break; break;
...@@ -123,7 +109,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -123,7 +109,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
sk->sk_state_change(sk); sk->sk_state_change(sk);
} }
} }
kfree_skb(skb);
sock_put(sk); sock_put(sk);
break; break;
case LLC_RESET_PRIM: case LLC_RESET_PRIM:
...@@ -132,14 +117,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -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 * RESET is not being notified to upper layers for now
*/ */
printk(KERN_INFO "%s: received a reset ind!\n", __func__); printk(KERN_INFO "%s: received a reset ind!\n", __func__);
kfree_skb(skb);
break; break;
default: default:
if (ev->ind_prim) { if (ev->ind_prim)
printk(KERN_INFO "%s: received unknown %d prim!\n", printk(KERN_INFO "%s: received unknown %d prim!\n",
__func__, ev->ind_prim); __func__, ev->ind_prim);
kfree_skb(skb);
}
/* No indication */ /* No indication */
break; break;
} }
...@@ -181,15 +163,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) ...@@ -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__); printk(KERN_INFO "%s: received a reset conf!\n", __func__);
break; break;
default: default:
if (ev->cfm_prim) { if (ev->cfm_prim)
printk(KERN_INFO "%s: received unknown %d prim!\n", printk(KERN_INFO "%s: received unknown %d prim!\n",
__func__, ev->cfm_prim); __func__, ev->cfm_prim);
/* No confirmation */
break; break;
} }
goto out_skb_put; /* No confirmation */
}
out_kfree_skb:
kfree_skb(skb);
out_skb_put: out_skb_put:
kfree_skb(skb); kfree_skb(skb);
return rc; return rc;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment