| Message ID | 1330593434-19275-1-git-send-email-santoshprasadnayak@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > I am getting following error. > " net/bridge/netfilter/ebtables.c:269 ebt_do_table() > error: potential null derefence 'cs'" > > i = cs[sp].n; // If cs == Null then this will cause problem. > > Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> > --- > net/bridge/netfilter/ebtables.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c > index f3fcbd9..9c0f177 100644 > --- a/net/bridge/netfilter/ebtables.c > +++ b/net/bridge/netfilter/ebtables.c > @@ -209,8 +209,10 @@ unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb, > smp_processor_id()); > if (private->chainstack) > cs = private->chainstack[smp_processor_id()]; > - else > + else { > cs = NULL; > + goto out; There is no "out" label in ebt_do_table !! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pablo, Please look at the last line of my patch. I have added a new label "out" regards santosh On Thu, Mar 1, 2012 at 3:51 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: >> From: Santosh Nayak <santoshprasadnayak@gmail.com> >> >> I am getting following error. >> " net/bridge/netfilter/ebtables.c:269 ebt_do_table() >> error: potential null derefence 'cs'" >> >> i = cs[sp].n; // If cs == Null then this will cause problem. >> >> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> >> --- >> net/bridge/netfilter/ebtables.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c >> index f3fcbd9..9c0f177 100644 >> --- a/net/bridge/netfilter/ebtables.c >> +++ b/net/bridge/netfilter/ebtables.c >> @@ -209,8 +209,10 @@ unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb, >> smp_processor_id()); >> if (private->chainstack) >> cs = private->chainstack[smp_processor_id()]; >> - else >> + else { >> cs = NULL; >> + goto out; > > There is no "out" label in ebt_do_table !! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > I am getting following error. > " net/bridge/netfilter/ebtables.c:269 ebt_do_table() > error: potential null derefence 'cs'" > > i = cs[sp].n; // If cs == Null then this will cause problem. Very sorry, I didn't see the out label. I'll apply this to my nf [1] once David takes my previous request for pulling. [1] http://1984.lsi.us.es/git/net -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > I am getting following error. > " net/bridge/netfilter/ebtables.c:269 ebt_do_table() > error: potential null derefence 'cs'" > > i = cs[sp].n; // If cs == Null then this will cause problem. > > Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> > --- > net/bridge/netfilter/ebtables.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c > index f3fcbd9..9c0f177 100644 > --- a/net/bridge/netfilter/ebtables.c > +++ b/net/bridge/netfilter/ebtables.c > @@ -209,8 +209,10 @@ unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb, > smp_processor_id()); > if (private->chainstack) > cs = private->chainstack[smp_processor_id()]; > - else > + else { > cs = NULL; I just noticed we can remove this cs = NULL. No need to resend, I'll mangle it myself. > + goto out; > + } > chaininfo = private->hook_entry[hook]; > nentries = private->hook_entry[hook]->nentries; > point = (struct ebt_entry *)(private->hook_entry[hook]->data); > @@ -313,6 +315,7 @@ letscontinue: > read_unlock_bh(&table->lock); > return NF_ACCEPT; > } > +out: > read_unlock_bh(&table->lock); > return NF_DROP; > } > -- > 1.7.4.4 > > -- > To unsubscribe from this list: send the line "unsubscribe netfilter" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Op 1/03/2012 13:30, Pablo Neira Ayuso schreef: > On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: >> From: Santosh Nayak<santoshprasadnayak@gmail.com> >> >> I am getting following error. >> " net/bridge/netfilter/ebtables.c:269 ebt_do_table() >> error: potential null derefence 'cs'" >> >> i = cs[sp].n; // If cs == Null then this will cause problem. > > Very sorry, I didn't see the out label. > > I'll apply this to my nf [1] once David takes my previous request for > pulling. > Hi, Has this patch been tested? Really, that code in the core firewall function is there for a reason, wouldn't you think? The chainstack is only allocated when user-defined chains are used (see translate_table). Never blindly trust a tool. Bart -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 02, 2012 at 10:31:23PM +0100, Bart De Schuymer wrote: > Op 1/03/2012 13:30, Pablo Neira Ayuso schreef: > >On Thu, Mar 01, 2012 at 02:47:14PM +0530, santosh nayak wrote: > >>From: Santosh Nayak<santoshprasadnayak@gmail.com> > >> > >>I am getting following error. > >>" net/bridge/netfilter/ebtables.c:269 ebt_do_table() > >> error: potential null derefence 'cs'" > >> > >> i = cs[sp].n; // If cs == Null then this will cause problem. > > > >Very sorry, I didn't see the out label. > > > >I'll apply this to my nf [1] once David takes my previous request for > >pulling. > > > > Hi, > > Has this patch been tested? Really, that code in the core firewall > function is there for a reason, wouldn't you think? > The chainstack is only allocated when user-defined chains are used > (see translate_table). > Never blindly trust a tool. I see, then that cs NULL dereference never happens. Thanks Bart, I'll drop this patch. -- To unsubscribe from this list: send the line "unsubscribe netdev" 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/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index f3fcbd9..9c0f177 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -209,8 +209,10 @@ unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb, smp_processor_id()); if (private->chainstack) cs = private->chainstack[smp_processor_id()]; - else + else { cs = NULL; + goto out; + } chaininfo = private->hook_entry[hook]; nentries = private->hook_entry[hook]->nentries; point = (struct ebt_entry *)(private->hook_entry[hook]->data); @@ -313,6 +315,7 @@ letscontinue: read_unlock_bh(&table->lock); return NF_ACCEPT; } +out: read_unlock_bh(&table->lock); return NF_DROP; }