* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning [not found] <20081123113315.GA16697@localhost> @ 2008-11-23 21:59 ` David Miller 2008-11-24 0:06 ` Wu Fengguang 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2008-11-23 21:59 UTC (permalink / raw) To: fengguang.wu; +Cc: kaber, linux-net, netdev From: Wu Fengguang <fengguang.wu@intel.com> Date: Sun, 23 Nov 2008 19:33:15 +0800 linux-net is not the mailing list for network development discussion, it is for user questions. Post patches and developer questions to netdev@vger.kernel.org instead. > net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > Signed-off-by: Wu Fengguang <wfg@linux.intel.com> > --- > > DISCLAIMER: I'm newbie to the code, and this fix could be wrong! > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c > index ae8c260..d31ced4 100644 > --- a/net/netfilter/nf_conntrack_proto_sctp.c > +++ b/net/netfilter/nf_conntrack_proto_sctp.c > @@ -373,6 +373,9 @@ static int sctp_packet(struct nf_conn *ct, > } > write_unlock_bh(&sctp_lock); > > + if (new_state == SCTP_CONNTRACK_MAX) > + goto out; > + > nf_ct_refresh_acct(ct, ctinfo, skb, sctp_timeouts[new_state]); > > if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED && > -- > To unsubscribe from this list: send the line "unsubscribe linux-net" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-23 21:59 ` [PATCH] netfilter: nf_conntrack_sctp: fix build warning David Miller @ 2008-11-24 0:06 ` Wu Fengguang 2008-11-24 1:08 ` Wang Chen 0 siblings, 1 reply; 8+ messages in thread From: Wu Fengguang @ 2008-11-24 0:06 UTC (permalink / raw) To: David Miller Cc: kaber@trash.net, linux-net@vger.kernel.org, netdev@vger.kernel.org On Sun, Nov 23, 2008 at 11:59:55PM +0200, David Miller wrote: > From: Wu Fengguang <fengguang.wu@intel.com> > Date: Sun, 23 Nov 2008 19:33:15 +0800 > > linux-net is not the mailing list for network development > discussion, it is for user questions. Post patches and > developer questions to netdev@vger.kernel.org instead. OK, thanks for the CC! Fengguang --- net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds Signed-off-by: Wu Fengguang <wfg@linux.intel.com> --- DISCLAIMER: I'm newbie to the code, and this fix could be wrong! diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c index ae8c260..d31ced4 100644 --- a/net/netfilter/nf_conntrack_proto_sctp.c +++ b/net/netfilter/nf_conntrack_proto_sctp.c @@ -373,6 +373,9 @@ static int sctp_packet(struct nf_conn *ct, } write_unlock_bh(&sctp_lock); + if (new_state == SCTP_CONNTRACK_MAX) + goto out; + nf_ct_refresh_acct(ct, ctinfo, skb, sctp_timeouts[new_state]); if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED && ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 0:06 ` Wu Fengguang @ 2008-11-24 1:08 ` Wang Chen 2008-11-24 1:17 ` Wu Fengguang 0 siblings, 1 reply; 8+ messages in thread From: Wang Chen @ 2008-11-24 1:08 UTC (permalink / raw) To: Wu Fengguang; +Cc: David Miller, kaber@trash.net, netdev@vger.kernel.org Wu Fengguang said the following on 2008-11-24 8:06: > On Sun, Nov 23, 2008 at 11:59:55PM +0200, David Miller wrote: >> From: Wu Fengguang <fengguang.wu@intel.com> >> Date: Sun, 23 Nov 2008 19:33:15 +0800 >> >> linux-net is not the mailing list for network development >> discussion, it is for user questions. Post patches and >> developer questions to netdev@vger.kernel.org instead. > > OK, thanks for the CC! > > Fengguang > --- > > net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > Signed-off-by: Wu Fengguang <wfg@linux.intel.com> > --- > > DISCLAIMER: I'm newbie to the code, and this fix could be wrong! > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c > index ae8c260..d31ced4 100644 > --- a/net/netfilter/nf_conntrack_proto_sctp.c > +++ b/net/netfilter/nf_conntrack_proto_sctp.c > @@ -373,6 +373,9 @@ static int sctp_packet(struct nf_conn *ct, > } > write_unlock_bh(&sctp_lock); > > + if (new_state == SCTP_CONNTRACK_MAX) > + goto out; > + print some debug info would be better? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 1:08 ` Wang Chen @ 2008-11-24 1:17 ` Wu Fengguang 2008-11-24 12:23 ` Patrick McHardy 0 siblings, 1 reply; 8+ messages in thread From: Wu Fengguang @ 2008-11-24 1:17 UTC (permalink / raw) To: Wang Chen; +Cc: David Miller, kaber@trash.net, netdev@vger.kernel.org On Mon, Nov 24, 2008 at 03:08:57AM +0200, Wang Chen wrote: > Wu Fengguang said the following on 2008-11-24 8:06: > > On Sun, Nov 23, 2008 at 11:59:55PM +0200, David Miller wrote: > >> From: Wu Fengguang <fengguang.wu@intel.com> > >> Date: Sun, 23 Nov 2008 19:33:15 +0800 > >> > >> linux-net is not the mailing list for network development > >> discussion, it is for user questions. Post patches and > >> developer questions to netdev@vger.kernel.org instead. > > > > OK, thanks for the CC! > > > > Fengguang > > --- > > > > net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: > > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > > > Signed-off-by: Wu Fengguang <wfg@linux.intel.com> > > --- > > > > DISCLAIMER: I'm newbie to the code, and this fix could be wrong! > > > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c > > index ae8c260..d31ced4 100644 > > --- a/net/netfilter/nf_conntrack_proto_sctp.c > > +++ b/net/netfilter/nf_conntrack_proto_sctp.c > > @@ -373,6 +373,9 @@ static int sctp_packet(struct nf_conn *ct, > > } > > write_unlock_bh(&sctp_lock); > > > > + if (new_state == SCTP_CONNTRACK_MAX) > > + goto out; > > + > > print some debug info would be better? Like this one? + pr_debug("Empty sctp packet\n"); Fengguang --- netfilter: nf_conntrack_sctp: fix build warning net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds Signed-off-by: Wu Fengguang <wfg@linux.intel.com> --- DISCLAIMER: I'm newbie to the code, and this fix could be wrong! diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c index ae8c260..a0bc24b 100644 --- a/net/netfilter/nf_conntrack_proto_sctp.c +++ b/net/netfilter/nf_conntrack_proto_sctp.c @@ -373,6 +373,11 @@ static int sctp_packet(struct nf_conn *ct, } write_unlock_bh(&sctp_lock); + if (new_state == SCTP_CONNTRACK_MAX) { + pr_debug("Empty sctp packet\n"); + goto out; + } + nf_ct_refresh_acct(ct, ctinfo, skb, sctp_timeouts[new_state]); if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED && ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 1:17 ` Wu Fengguang @ 2008-11-24 12:23 ` Patrick McHardy 2008-11-24 12:35 ` Wu Fengguang 0 siblings, 1 reply; 8+ messages in thread From: Patrick McHardy @ 2008-11-24 12:23 UTC (permalink / raw) To: Wu Fengguang Cc: Wang Chen, David Miller, netdev@vger.kernel.org, Netfilter Development Mailinglist [-- Attachment #1: Type: text/plain, Size: 527 bytes --] Wu Fengguang wrote: >> print some debug info would be better? > > Like this one? > + pr_debug("Empty sctp packet\n"); > > Fengguang > --- > netfilter: nf_conntrack_sctp: fix build warning > > net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds The warning is bogus, so we don't need a pr_debug() there. I've applied your first patch with an unlikely() added and a comment stating that the warning is bogus. [-- Attachment #2: x --] [-- Type: text/plain, Size: 1173 bytes --] commit a3e2913ffd3dec7f6975f680035670261ad5f56e Author: Wu Fengguang <wfg@linux.intel.com> Date: Mon Nov 24 13:18:00 2008 +0100 netfilter: nf_conntrack_sctp: fix build warning net/netfilter/nf_conntrack_proto_sctp.c: In function 'sctp_packet': net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds [Patrick; add unlikely and comment stating that the warning is bogus] Signed-off-by: Wu Fengguang <wfg@linux.intel.com> Signed-off-by: Patrick McHardy <kaber@trash.net> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c index c2bd457..1259ec6 100644 --- a/net/netfilter/nf_conntrack_proto_sctp.c +++ b/net/netfilter/nf_conntrack_proto_sctp.c @@ -373,6 +373,12 @@ static int sctp_packet(struct nf_conn *ct, } write_unlock_bh(&sctp_lock); + /* Avoid bogus warning, gcc doesn't realize do_basic_checks() + * guarantees that there is at least one SCTP chunk. + */ + if (unlikely(new_state == SCTP_CONNTRACK_MAX)) + goto out; + nf_ct_refresh_acct(ct, ctinfo, skb, sctp_timeouts[new_state]); if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED && ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 12:23 ` Patrick McHardy @ 2008-11-24 12:35 ` Wu Fengguang 2008-11-24 12:48 ` Patrick McHardy 0 siblings, 1 reply; 8+ messages in thread From: Wu Fengguang @ 2008-11-24 12:35 UTC (permalink / raw) To: Patrick McHardy Cc: Wang Chen, David Miller, netdev@vger.kernel.org, Netfilter Development Mailinglist On Mon, Nov 24, 2008 at 02:23:16PM +0200, Patrick McHardy wrote: > Wu Fengguang wrote: > >> print some debug info would be better? > > > > Like this one? > > + pr_debug("Empty sctp packet\n"); > > > > Fengguang > > --- > > netfilter: nf_conntrack_sctp: fix build warning > > > > net/netfilter/nf_conntrack_proto_sctp.c: In function ‘sctp_packet’: > > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > The warning is bogus, so we don't need a pr_debug() there. > I've applied your first patch with an unlikely() added and > a comment stating that the warning is bogus. Thank you! I was expecting some better solution (and learn a bit) ;-) > commit a3e2913ffd3dec7f6975f680035670261ad5f56e > Author: Wu Fengguang <wfg@linux.intel.com> > Date: Mon Nov 24 13:18:00 2008 +0100 > > netfilter: nf_conntrack_sctp: fix build warning > > net/netfilter/nf_conntrack_proto_sctp.c: In function 'sctp_packet': > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > [Patrick; add unlikely and comment stating that the warning is bogus] > > Signed-off-by: Wu Fengguang <wfg@linux.intel.com> > Signed-off-by: Patrick McHardy <kaber@trash.net> > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c > index c2bd457..1259ec6 100644 > --- a/net/netfilter/nf_conntrack_proto_sctp.c > +++ b/net/netfilter/nf_conntrack_proto_sctp.c > @@ -373,6 +373,12 @@ static int sctp_packet(struct nf_conn *ct, > } > write_unlock_bh(&sctp_lock); > > + /* Avoid bogus warning, gcc doesn't realize do_basic_checks() > + * guarantees that there is at least one SCTP chunk. > + */ > + if (unlikely(new_state == SCTP_CONNTRACK_MAX)) > + goto out; > + If do_basic_checks() guarantees that, why not Initialize new_state to 0? --- diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c index ae8c260..218137d 100644 --- a/net/netfilter/nf_conntrack_proto_sctp.c +++ b/net/netfilter/nf_conntrack_proto_sctp.c @@ -317,7 +317,11 @@ static int sctp_packet(struct nf_conn *ct, goto out; } - old_state = new_state = SCTP_CONNTRACK_MAX; + /* Avoid bogus warning, gcc doesn't realize do_basic_checks() + * guarantees that there is at least one SCTP chunk. + */ + old_state = new_state = 0; + write_lock_bh(&sctp_lock); for_each_sctp_chunk (skb, sch, _sch, offset, dataoff, count) { /* Special cases of Verification tag check (Sec 8.5.1) */ -- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 12:35 ` Wu Fengguang @ 2008-11-24 12:48 ` Patrick McHardy 2008-11-24 12:50 ` Wu Fengguang 0 siblings, 1 reply; 8+ messages in thread From: Patrick McHardy @ 2008-11-24 12:48 UTC (permalink / raw) To: Wu Fengguang Cc: Wang Chen, David Miller, netdev@vger.kernel.org, Netfilter Development Mailinglist [-- Attachment #1: Type: text/plain, Size: 423 bytes --] Wu Fengguang wrote: > On Mon, Nov 24, 2008 at 02:23:16PM +0200, Patrick McHardy wrote: >> + /* Avoid bogus warning, gcc doesn't realize do_basic_checks() >> + * guarantees that there is at least one SCTP chunk. >> + */ >> + if (unlikely(new_state == SCTP_CONNTRACK_MAX)) >> + goto out; >> + > > If do_basic_checks() guarantees that, why not Initialize new_state to 0? Good point. I've replaced the patch by this one: [-- Attachment #2: x --] [-- Type: text/plain, Size: 1260 bytes --] commit 328bd8997dbb7184d5389e45c642af44ae6e9043 Author: Patrick McHardy <kaber@trash.net> Date: Mon Nov 24 13:44:55 2008 +0100 netfilter: nf_conntrack_proto_sctp: avoid bogus warning net/netfilter/nf_conntrack_proto_sctp.c: In function 'sctp_packet': net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds gcc doesn't realize that do_basic_checks() guarantees that there is at least one valid chunk and thus new_state is never SCTP_CONNTRACK_MAX after the loop. Initialize to SCTP_CONNTRACK_NONE to avoid the warning. Based on patch by Wu Fengguang <wfg@linux.intel.com> Signed-off-by: Patrick McHardy <kaber@trash.net> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c index c2bd457..74e0379 100644 --- a/net/netfilter/nf_conntrack_proto_sctp.c +++ b/net/netfilter/nf_conntrack_proto_sctp.c @@ -317,7 +317,7 @@ static int sctp_packet(struct nf_conn *ct, goto out; } - old_state = new_state = SCTP_CONNTRACK_MAX; + old_state = new_state = SCTP_CONNTRACK_NONE; write_lock_bh(&sctp_lock); for_each_sctp_chunk (skb, sch, _sch, offset, dataoff, count) { /* Special cases of Verification tag check (Sec 8.5.1) */ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 12:48 ` Patrick McHardy @ 2008-11-24 12:50 ` Wu Fengguang 0 siblings, 0 replies; 8+ messages in thread From: Wu Fengguang @ 2008-11-24 12:50 UTC (permalink / raw) To: Patrick McHardy Cc: Wang Chen, David Miller, netdev@vger.kernel.org, Netfilter Development Mailinglist On Mon, Nov 24, 2008 at 02:48:07PM +0200, Patrick McHardy wrote: > Wu Fengguang wrote: > > On Mon, Nov 24, 2008 at 02:23:16PM +0200, Patrick McHardy wrote: > >> + /* Avoid bogus warning, gcc doesn't realize do_basic_checks() > >> + * guarantees that there is at least one SCTP chunk. > >> + */ > >> + if (unlikely(new_state == SCTP_CONNTRACK_MAX)) > >> + goto out; > >> + > > > > If do_basic_checks() guarantees that, why not Initialize new_state to 0? > > Good point. I've replaced the patch by this one: Thanks! Fengguang Content-Description: x > commit 328bd8997dbb7184d5389e45c642af44ae6e9043 > Author: Patrick McHardy <kaber@trash.net> > Date: Mon Nov 24 13:44:55 2008 +0100 > > netfilter: nf_conntrack_proto_sctp: avoid bogus warning > > net/netfilter/nf_conntrack_proto_sctp.c: In function 'sctp_packet': > net/netfilter/nf_conntrack_proto_sctp.c:376: warning: array subscript is above array bounds > > gcc doesn't realize that do_basic_checks() guarantees that there is > at least one valid chunk and thus new_state is never SCTP_CONNTRACK_MAX > after the loop. Initialize to SCTP_CONNTRACK_NONE to avoid the warning. > > Based on patch by Wu Fengguang <wfg@linux.intel.com> > > Signed-off-by: Patrick McHardy <kaber@trash.net> > > diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c > index c2bd457..74e0379 100644 > --- a/net/netfilter/nf_conntrack_proto_sctp.c > +++ b/net/netfilter/nf_conntrack_proto_sctp.c > @@ -317,7 +317,7 @@ static int sctp_packet(struct nf_conn *ct, > goto out; > } > > - old_state = new_state = SCTP_CONNTRACK_MAX; > + old_state = new_state = SCTP_CONNTRACK_NONE; > write_lock_bh(&sctp_lock); > for_each_sctp_chunk (skb, sch, _sch, offset, dataoff, count) { > /* Special cases of Verification tag check (Sec 8.5.1) */ ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-11-24 12:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20081123113315.GA16697@localhost>
2008-11-23 21:59 ` [PATCH] netfilter: nf_conntrack_sctp: fix build warning David Miller
2008-11-24 0:06 ` Wu Fengguang
2008-11-24 1:08 ` Wang Chen
2008-11-24 1:17 ` Wu Fengguang
2008-11-24 12:23 ` Patrick McHardy
2008-11-24 12:35 ` Wu Fengguang
2008-11-24 12:48 ` Patrick McHardy
2008-11-24 12:50 ` Wu Fengguang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).