| Message ID | 1406733475-5222-2-git-send-email-a.perevalov@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Jul 30, 2014 at 07:17:54PM +0400, Alexey Perevalov wrote: > Bit helper functions were used for manipulation with NFACCT_F_OVERQUOTA, > but they are accepting pit position, but not a bit mask. As a result > not a third bit for NFACCT_F_OVERQUOTA was set, but forth. Such > behaviour was dangarous and could lead to unexpected overquota report > result. > > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com> > --- > net/netfilter/nfnetlink_acct.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c > index 2baa125..127d24e 100644 > --- a/net/netfilter/nfnetlink_acct.c > +++ b/net/netfilter/nfnetlink_acct.c > @@ -77,7 +77,7 @@ nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb, > smp_mb__before_atomic(); > /* reset overquota flag if quota is enabled. */ > if ((matching->flags & NFACCT_F_QUOTA)) > - clear_bit(NFACCT_F_OVERQUOTA, &matching->flags); > + matching->flags &= ~NFACCT_F_OVERQUOTA; > return 0; > } > return -EBUSY; > @@ -148,7 +148,7 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type, > bytes = atomic64_xchg(&acct->bytes, 0); > smp_mb__before_atomic(); > if (acct->flags & NFACCT_F_QUOTA) > - clear_bit(NFACCT_F_OVERQUOTA, &acct->flags); > + acct->flags &= ~NFACCT_F_OVERQUOTA; > } else { > pkts = atomic64_read(&acct->pkts); > bytes = atomic64_read(&acct->bytes); > @@ -411,8 +411,8 @@ int nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct) > > ret = now > *quota; > > - if (now >= *quota && > - !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags)) { We cannot do this. The aim of that code it to avoid to deliver several overquota notification when several cpus are racing to update the counters and check if they have go over the quota. I think you have to define NFACCT_F_*_BIT and use it from the bitwise functions to fix this. You can define these in: include/uapi/linux/netfilter/nfnetlink_acct.h And use them to define the enum nfnl_acct_flags. 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
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c index 2baa125..127d24e 100644 --- a/net/netfilter/nfnetlink_acct.c +++ b/net/netfilter/nfnetlink_acct.c @@ -77,7 +77,7 @@ nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb, smp_mb__before_atomic(); /* reset overquota flag if quota is enabled. */ if ((matching->flags & NFACCT_F_QUOTA)) - clear_bit(NFACCT_F_OVERQUOTA, &matching->flags); + matching->flags &= ~NFACCT_F_OVERQUOTA; return 0; } return -EBUSY; @@ -148,7 +148,7 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type, bytes = atomic64_xchg(&acct->bytes, 0); smp_mb__before_atomic(); if (acct->flags & NFACCT_F_QUOTA) - clear_bit(NFACCT_F_OVERQUOTA, &acct->flags); + acct->flags &= ~NFACCT_F_OVERQUOTA; } else { pkts = atomic64_read(&acct->pkts); bytes = atomic64_read(&acct->bytes); @@ -411,8 +411,8 @@ int nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct) ret = now > *quota; - if (now >= *quota && - !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags)) { + if (now >= *quota) { + nfacct->flags |= NFACCT_F_OVERQUOTA; nfnl_overquota_report(nfacct); }
Bit helper functions were used for manipulation with NFACCT_F_OVERQUOTA, but they are accepting pit position, but not a bit mask. As a result not a third bit for NFACCT_F_OVERQUOTA was set, but forth. Such behaviour was dangarous and could lead to unexpected overquota report result. Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com> --- net/netfilter/nfnetlink_acct.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)