On 31 July 2015 at 17:40, Florian Westphal <fw@xxxxxxxxx> wrote:
> Joe Stringer <joestringer@xxxxxxxxxx> 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
Thanks for the review, I'll fix these up and send a v2 soon.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
