| Message ID | 20181215182504.26001-1-phil@nwl.cc |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [iptables] xtables: Catch errors when zeroing rule rounters | expand |
On Sat, Dec 15, 2018 at 07:25:04PM +0100, Phil Sutter wrote: > Covscan complained about call to batch_rule_add() not being checked. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > iptables/nft.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/iptables/nft.c b/iptables/nft.c > index f9024927c0e73..ea7419b620e7a 100644 > --- a/iptables/nft.c > +++ b/iptables/nft.c > @@ -3022,7 +3022,9 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data) > * rule based on its handle only. > */ > nftnl_rule_unset(r, NFTNL_RULE_POSITION); > - batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > + ret = batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > + if (ret) > + return -1; Probably 'goto err' so we destroy the iterator before returning -1? > } > r = nftnl_rule_iter_next(iter); > } > -- > 2.19.0 >
On Fri, Dec 21, 2018 at 12:24:55PM +0100, Pablo Neira Ayuso wrote: > On Sat, Dec 15, 2018 at 07:25:04PM +0100, Phil Sutter wrote: > > Covscan complained about call to batch_rule_add() not being checked. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > --- > > iptables/nft.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/iptables/nft.c b/iptables/nft.c > > index f9024927c0e73..ea7419b620e7a 100644 > > --- a/iptables/nft.c > > +++ b/iptables/nft.c > > @@ -3022,7 +3022,9 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data) > > * rule based on its handle only. > > */ > > nftnl_rule_unset(r, NFTNL_RULE_POSITION); > > - batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > > + ret = batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > > + if (ret) > > + return -1; > > Probably 'goto err' so we destroy the iterator before returning -1? Let me get to review your larger pending patchset, then you can resubmit this on top. Sorry I didn't get to this any sooner. > > } > > r = nftnl_rule_iter_next(iter); > > } > > -- > > 2.19.0 > >
Hi Pablo, On Fri, Dec 21, 2018 at 12:35:38PM +0100, Pablo Neira Ayuso wrote: > On Fri, Dec 21, 2018 at 12:24:55PM +0100, Pablo Neira Ayuso wrote: > > On Sat, Dec 15, 2018 at 07:25:04PM +0100, Phil Sutter wrote: > > > Covscan complained about call to batch_rule_add() not being checked. > > > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > > --- > > > iptables/nft.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/iptables/nft.c b/iptables/nft.c > > > index f9024927c0e73..ea7419b620e7a 100644 > > > --- a/iptables/nft.c > > > +++ b/iptables/nft.c > > > @@ -3022,7 +3022,9 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data) > > > * rule based on its handle only. > > > */ > > > nftnl_rule_unset(r, NFTNL_RULE_POSITION); > > > - batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > > > + ret = batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); > > > + if (ret) > > > + return -1; > > > > Probably 'goto err' so we destroy the iterator before returning -1? Oh, right. I Missed that. > Let me get to review your larger pending patchset, then you can > resubmit this on top. > > Sorry I didn't get to this any sooner. Thanks! And no worries, we all are busy with multiple things at once. :) Cheers, Phil
diff --git a/iptables/nft.c b/iptables/nft.c index f9024927c0e73..ea7419b620e7a 100644 --- a/iptables/nft.c +++ b/iptables/nft.c @@ -3022,7 +3022,9 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data) * rule based on its handle only. */ nftnl_rule_unset(r, NFTNL_RULE_POSITION); - batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); + ret = batch_rule_add(h, NFT_COMPAT_RULE_REPLACE, r); + if (ret) + return -1; } r = nftnl_rule_iter_next(iter); }
Covscan complained about call to batch_rule_add() not being checked. Signed-off-by: Phil Sutter <phil@nwl.cc> --- iptables/nft.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)