| Message ID | 20120920135133.GA21538@uweber-WS |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, 2012-09-20 at 15:52 +0200, Ulrich Weber wrote: > hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), > so rcu_read_unlock() is unnecessary if in_range() matches. > > Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com> > --- > net/netfilter/nf_nat_core.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > index 1816ad3..125d832 100644 > --- a/net/netfilter/nf_nat_core.c > +++ b/net/netfilter/nf_nat_core.c > @@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone, > &ct->tuplehash[IP_CT_DIR_REPLY].tuple); > result->dst = tuple->dst; > > - if (in_range(l3proto, l4proto, result, range)) { > - rcu_read_unlock(); > + if (in_range(l3proto, l4proto, result, range)) > return 1; > - } > } > } > return 0; This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next) How old is this bug ? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/20/2012 04:14 PM, Eric Dumazet wrote: > This could explain the rcu_read_unlock() imbalance Stephen had > (lockdep splat with net-next) How old is this bug ? Bug was introduced with c7232c9979cba684c50b64c513c4a83c9aa70563 Cheers Ulrich
On Thu, 20 Sep 2012 16:14:45 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Thu, 2012-09-20 at 15:52 +0200, Ulrich Weber wrote: > > hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), > > so rcu_read_unlock() is unnecessary if in_range() matches. > > > > Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com> > > --- > > net/netfilter/nf_nat_core.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > > index 1816ad3..125d832 100644 > > --- a/net/netfilter/nf_nat_core.c > > +++ b/net/netfilter/nf_nat_core.c > > @@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone, > > &ct->tuplehash[IP_CT_DIR_REPLY].tuple); > > result->dst = tuple->dst; > > > > - if (in_range(l3proto, l4proto, result, range)) { > > - rcu_read_unlock(); > > + if (in_range(l3proto, l4proto, result, range)) > > return 1; > > - } > > } > > } > > return 0; > > > This could explain the rcu_read_unlock() imbalance Stephen had (lockdep > splat with net-next) > > > How old is this bug ? > > > Will validate the fix tomorrow when back home. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 20, 2012 at 03:52:04PM +0200, Ulrich Weber wrote: > hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), > so rcu_read_unlock() is unnecessary if in_range() matches. Applied, thanks Ulrich. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 20, 2012 at 04:17:25PM +0200, Ulrich Weber wrote: > On 09/20/2012 04:14 PM, Eric Dumazet wrote: > >This could explain the rcu_read_unlock() imbalance Stephen had > >(lockdep splat with net-next) How old is this bug ? > > Bug was introduced with c7232c9979cba684c50b64c513c4a83c9aa70563 I've added this to the patch description, it's good for the record. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 20 Sep 2012 16:14:45 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Thu, 2012-09-20 at 15:52 +0200, Ulrich Weber wrote: > > hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), > > so rcu_read_unlock() is unnecessary if in_range() matches. > > > > Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com> > > --- > > net/netfilter/nf_nat_core.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > > index 1816ad3..125d832 100644 > > --- a/net/netfilter/nf_nat_core.c > > +++ b/net/netfilter/nf_nat_core.c > > @@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone, > > &ct->tuplehash[IP_CT_DIR_REPLY].tuple); > > result->dst = tuple->dst; > > > > - if (in_range(l3proto, l4proto, result, range)) { > > - rcu_read_unlock(); > > + if (in_range(l3proto, l4proto, result, range)) > > return 1; > > - } > > } > > } > > return 0; > > > This could explain the rcu_read_unlock() imbalance Stephen had (lockdep > splat with net-next) > > > How old is this bug ? > > > This fixes the lockdep splat I saw when staring KVM vm's with net-next and lockdep enabled -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 21, 2012 at 10:08:42AM -0700, Stephen Hemminger wrote: > On Thu, 20 Sep 2012 16:14:45 +0200 > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > On Thu, 2012-09-20 at 15:52 +0200, Ulrich Weber wrote: > > > hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), > > > so rcu_read_unlock() is unnecessary if in_range() matches. > > > > > > Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com> > > > --- > > > net/netfilter/nf_nat_core.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > > > index 1816ad3..125d832 100644 > > > --- a/net/netfilter/nf_nat_core.c > > > +++ b/net/netfilter/nf_nat_core.c > > > @@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone, > > > &ct->tuplehash[IP_CT_DIR_REPLY].tuple); > > > result->dst = tuple->dst; > > > > > > - if (in_range(l3proto, l4proto, result, range)) { > > > - rcu_read_unlock(); > > > + if (in_range(l3proto, l4proto, result, range)) > > > return 1; > > > - } > > > } > > > } > > > return 0; > > > > > > This could explain the rcu_read_unlock() imbalance Stephen had (lockdep > > splat with net-next) > > > > > > How old is this bug ? > > > > > > > > This fixes the lockdep splat I saw when staring KVM vm's with net-next and lockdep enabled It's in my tree already. I'll pass it to David asap. Thanks for the report. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 1816ad3..125d832 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone, &ct->tuplehash[IP_CT_DIR_REPLY].tuple); result->dst = tuple->dst; - if (in_range(l3proto, l4proto, result, range)) { - rcu_read_unlock(); + if (in_range(l3proto, l4proto, result, range)) return 1; - } } } return 0;
hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches. Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com> --- net/netfilter/nf_nat_core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)