* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning [not found] ` <20081124011727.GA6462@localhost> @ 2008-11-24 12:23 ` Patrick McHardy 2008-11-24 12:35 ` Wu Fengguang 0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* Re: [PATCH] netfilter: nf_conntrack_sctp: fix build warning 2008-11-24 12:23 ` [PATCH] netfilter: nf_conntrack_sctp: fix build warning Patrick McHardy @ 2008-11-24 12:35 ` Wu Fengguang 2008-11-24 12:48 ` Patrick McHardy 0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2008-11-24 12:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20081123113315.GA16697@localhost>
[not found] ` <20081123.135955.202482968.davem@davemloft.net>
[not found] ` <20081124000652.GA5424@localhost>
[not found] ` <4929FEA9.9090801@cn.fujitsu.com>
[not found] ` <20081124011727.GA6462@localhost>
2008-11-24 12:23 ` [PATCH] netfilter: nf_conntrack_sctp: fix build warning 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).