| From 32508e091abf9b6f158d14b3086d72578b8fdb61 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sun, 6 Oct 2019 14:24:27 -0700 |
| Subject: llc: fix sk_buff refcounting in llc_conn_state_process() |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| [ Upstream commit 36453c852816f19947ca482a595dffdd2efa4965 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/llc/llc_conn.c | 33 ++++++--------------------------- |
| 1 file changed, 6 insertions(+), 27 deletions(-) |
| |
| diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c |
| index 2689e95471dc4..1bdbd134bd7a8 100644 |
| --- 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; |
| -- |
| 2.20.1 |
| |