Re: [PATCH nf-next] netfilter: connlabels: Export setting connlabel length

[Florian Westphal <fw@strlen.de>]

Joe Stringer <joestringer@nicira.com> wrote:
> diff --git a/net/netfilter/nf_conntrack_labels.c b/net/netfilter/nf_conntrack_labels.c
> index bb53f12..00df4e71 100644
> --- a/net/netfilter/nf_conntrack_labels.c
> +++ b/net/netfilter/nf_conntrack_labels.c
> @@ -91,6 +91,30 @@ int nf_connlabels_replace(struct nf_conn *ct,
>  EXPORT_SYMBOL_GPL(nf_connlabels_replace);
>  #endif
>  
> +int nf_connlabels_get(struct net *net, unsigned int n_bits)
> +{
> +	size_t words;
> +
> +	if (n_bits > XT_CONNLABEL_MAXBIT + 1)

Perhaps use

  if (n_bits >= (NF_CT_LABELS_MAX_SIZE * BITS_PER_BYTE)))

To avoid the XT_CONNLABEL_MAXBIT in the nf_* part.

> +		return -ERANGE;
> +
> +	net->ct.labels_used++;
> +	words = BITS_TO_LONGS(n_bits);
> +	if (words > net->ct.label_words)
> +		net->ct.label_words = words;
> +
> +	return 0;
> +}

I think we should add a lock here, currently this is protected by the
xtables mutex -- I'd suggest to just add a spinlock for this.

>  		return ret;
>  	}
>  
> -	par->net->ct.labels_used++;
> -	words = BITS_TO_LONGS(info->bit+1);
> -	if (words > par->net->ct.label_words)
> -		par->net->ct.label_words = words;
> -
> -	return ret;
> +	return nf_connlabels_get(par->net, info->bit + 1);

This can leak the refcnt on l3_proto_module we obtained earlier.

Other than that, this looks good.

Thanks,
Florian