lkml.org 
[lkml]   [2010]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: Regression, bisected: reference leak with IPSec since ~2.6.31
    From
    Date
    Le mardi 21 septembre 2010 à 09:12 +0000, Jarek Poplawski a écrit :
    > On 2010-09-20 23:31, Eric Dumazet wrote:
    > ...
    > > [PATCH] ip : fix truesize mismatch in ip fragmentation
    > >
    > > We should not set frag->destructor to sock_wkfree() until we are sure we
    > > dont hit slow path in ip_fragment(). Or we risk uncharging
    > > frag->truesize twice, and in the end, having negative socket
    > > sk_wmem_alloc counter, or even freeing socket sooner than expected.
    > >
    > > Many thanks to Nick Bowler, who provided a very clean bug report and
    > > test programs.
    > >
    > > While Nick bisection pointed to commit 2b85a34e911bf483 (net: No more
    > > expensive sock_hold()/sock_put() on each tx), underlying bug is older.
    > >
    > > Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
    > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    > > ---
    > > net/ipv4/ip_output.c | 8 ++++----
    > > net/ipv6/ip6_output.c | 10 +++++-----
    > > 2 files changed, 9 insertions(+), 9 deletions(-)
    > >
    > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
    > > index 04b6989..126d9b3 100644
    > > --- a/net/ipv4/ip_output.c
    > > +++ b/net/ipv4/ip_output.c
    > > @@ -490,7 +490,6 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
    > > if (skb_has_frags(skb)) {
    > > struct sk_buff *frag;
    > > int first_len = skb_pagelen(skb);
    > > - int truesizes = 0;
    > >
    > > if (first_len - hlen > mtu ||
    > > ((first_len - hlen) & 7) ||
    > > @@ -510,11 +509,13 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
    > > goto slow_path;
    > >
    > > BUG_ON(frag->sk);
    > > - if (skb->sk) {
    > > + }
    > > + if (skb->sk) {
    > > + skb_walk_frags(skb, frag) {
    > > frag->sk = skb->sk;
    > > frag->destructor = sock_wfree;
    >
    > Nice catch, but it seems doing it in the first loop as now, and
    > reverting changes before goto slow_path might be more optimal here.
    >

    I thought of this, but found this function already very complex.

    Once everything is in cpu caches, the added loop is very cheap.

    I liked the :

    <check everything without changing state>
    if something wrong
    goto slow_path
    else
    <OK, lets do destructive things>


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-09-21 11:23    [W:3.052 / U:0.204 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site