[ovs-dev,v2,1/2] datapath: Fix refcount leak on force commit.

Submitted by Jarno Rajahalme on April 19, 2017, 11:54 p.m.

Details

Message ID 1492646078-55043-1-git-send-email-jarno@ovn.org
State New
Headers show

Commit Message

Jarno Rajahalme April 19, 2017, 11:54 p.m.
Upstream commit:

    commit b768b16de58d5e0b1d7c3f936825b25327ced20c
    Author: Jarno Rajahalme <jarno@ovn.org>
    Date:   Tue Mar 28 11:25:26 2017 -0700

    openvswitch: Fix refcount leak on force commit.

    The reference count held for skb needs to be released when the skb's
    nfct pointer is cleared regardless of if nf_ct_delete() is called or
    not.

    Failing to release the skb's reference cound led to deferred conntrack
    cleanup spinning forever within nf_conntrack_cleanup_net_list() when
    cleaning up a network namespace:

       kworker/u16:0-19025 [004] 45981067.173642: sched_switch: kworker/u16:0:19025 [120] R ==> rcu_preempt:7 [120]
       kworker/u16:0-19025 [004] 45981067.173651: kernel_stack: <stack trace>
    => ___preempt_schedule (ffffffffa001ed36)
    => _raw_spin_unlock_bh (ffffffffa0713290)
    => nf_ct_iterate_cleanup (ffffffffc00a4454)
    => nf_conntrack_cleanup_net_list (ffffffffc00a5e1e)
    => nf_conntrack_pernet_exit (ffffffffc00a63dd)
    => ops_exit_list.isra.1 (ffffffffa06075f3)
    => cleanup_net (ffffffffa0607df0)
    => process_one_work (ffffffffa0084c31)
    => worker_thread (ffffffffa008592b)
    => kthread (ffffffffa008bee2)
    => ret_from_fork (ffffffffa071b67c)

    Fixes: dd41d33f0b03 ("openvswitch: Add force commit.")
    Reported-by: Yang Song <yangsong@vmware.com>
    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
    Acked-by: Joe Stringer <joe@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
v2: No change.

datapath/conntrack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Joe Stringer April 20, 2017, 7:40 p.m.
On 19 April 2017 at 16:54, Jarno Rajahalme <jarno@ovn.org> wrote:
> Upstream commit:
>
>     commit b768b16de58d5e0b1d7c3f936825b25327ced20c
>     Author: Jarno Rajahalme <jarno@ovn.org>
>     Date:   Tue Mar 28 11:25:26 2017 -0700
>
>     openvswitch: Fix refcount leak on force commit.
>
>     The reference count held for skb needs to be released when the skb's
>     nfct pointer is cleared regardless of if nf_ct_delete() is called or
>     not.
>
>     Failing to release the skb's reference cound led to deferred conntrack
>     cleanup spinning forever within nf_conntrack_cleanup_net_list() when
>     cleaning up a network namespace:
>
>        kworker/u16:0-19025 [004] 45981067.173642: sched_switch: kworker/u16:0:19025 [120] R ==> rcu_preempt:7 [120]
>        kworker/u16:0-19025 [004] 45981067.173651: kernel_stack: <stack trace>
>     => ___preempt_schedule (ffffffffa001ed36)
>     => _raw_spin_unlock_bh (ffffffffa0713290)
>     => nf_ct_iterate_cleanup (ffffffffc00a4454)
>     => nf_conntrack_cleanup_net_list (ffffffffc00a5e1e)
>     => nf_conntrack_pernet_exit (ffffffffc00a63dd)
>     => ops_exit_list.isra.1 (ffffffffa06075f3)
>     => cleanup_net (ffffffffa0607df0)
>     => process_one_work (ffffffffa0084c31)
>     => worker_thread (ffffffffa008592b)
>     => kthread (ffffffffa008bee2)
>     => ret_from_fork (ffffffffa071b67c)
>
>     Fixes: dd41d33f0b03 ("openvswitch: Add force commit.")
>     Reported-by: Yang Song <yangsong@vmware.com>
>     Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>     Acked-by: Joe Stringer <joe@ovn.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Acked-by: Joe Stringer <joe@ovn.org>
Jarno Rajahalme April 21, 2017, 1:15 a.m.
> On Apr 20, 2017, at 12:40 PM, Joe Stringer <joe@ovn.org> wrote:
> 
> On 19 April 2017 at 16:54, Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>> wrote:
>> Upstream commit:
>> 
>>    commit b768b16de58d5e0b1d7c3f936825b25327ced20c
>>    Author: Jarno Rajahalme <jarno@ovn.org>
>>    Date:   Tue Mar 28 11:25:26 2017 -0700
>> 
>>    openvswitch: Fix refcount leak on force commit.
>> 
>>    The reference count held for skb needs to be released when the skb's
>>    nfct pointer is cleared regardless of if nf_ct_delete() is called or
>>    not.
>> 
>>    Failing to release the skb's reference cound led to deferred conntrack
>>    cleanup spinning forever within nf_conntrack_cleanup_net_list() when
>>    cleaning up a network namespace:
>> 
>>       kworker/u16:0-19025 [004] 45981067.173642: sched_switch: kworker/u16:0:19025 [120] R ==> rcu_preempt:7 [120]
>>       kworker/u16:0-19025 [004] 45981067.173651: kernel_stack: <stack trace>
>>    => ___preempt_schedule (ffffffffa001ed36)
>>    => _raw_spin_unlock_bh (ffffffffa0713290)
>>    => nf_ct_iterate_cleanup (ffffffffc00a4454)
>>    => nf_conntrack_cleanup_net_list (ffffffffc00a5e1e)
>>    => nf_conntrack_pernet_exit (ffffffffc00a63dd)
>>    => ops_exit_list.isra.1 (ffffffffa06075f3)
>>    => cleanup_net (ffffffffa0607df0)
>>    => process_one_work (ffffffffa0084c31)
>>    => worker_thread (ffffffffa008592b)
>>    => kthread (ffffffffa008bee2)
>>    => ret_from_fork (ffffffffa071b67c)
>> 
>>    Fixes: dd41d33f0b03 ("openvswitch: Add force commit.")
>>    Reported-by: Yang Song <yangsong@vmware.com>
>>    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>    Acked-by: Joe Stringer <joe@ovn.org>
>>    Signed-off-by: David S. Miller <davem@davemloft.net>
>> 
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> 
> Acked-by: Joe Stringer <joe@ovn.org <mailto:joe@ovn.org>>

Thanks, pushed to master,

  Jarno

Patch hide | download patch | download mbox

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 109b297..4c42a48 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -677,8 +677,8 @@  static bool skb_nfct_cached(struct net *net,
 		 */
 		if (nf_ct_is_confirmed(ct))
 			nf_ct_delete(ct, 0, 0);
-		else
-			nf_conntrack_put(&ct->ct_general);
+
+		nf_conntrack_put(&ct->ct_general);
 		nf_ct_set(skb, NULL, 0);
 		return false;
 	}