Re: [PATCH 1/2] netfilter: nfnetlink_acct: avoid using NFACCT_F_OVERQUOTA with bit helper funcitons

[Pablo Neira Ayuso <pablo@netfilter.org>]

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.