On 15 October 2015 at 11:13, Pravin Shelar <pshe...@nicira.com> wrote:
> On Wed, Oct 14, 2015 at 11:10 AM, Joe Stringer <joestrin...@nicira.com> wrote:
>> If userspace provides a ct action with no nested mark or label, then the
>> storage for these fields is zeroed. Later when actions are requested,
>> such zeroed fields are serialized even though userspace didn't
>> originally specify them. Fix the behaviour by ensuring that no action is
>> serialized in this case, and reject actions where userspace attempts to
>> set these fields with mask=0. This should make netlink marshalling
>> consistent across deserialization/reserialization.
>>
>> Reported-by: Jarno Rajahalme <jrajaha...@nicira.com>
>> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
>> ---
>> net/openvswitch/conntrack.c | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> index 480dbb9095b7..ba29e6c2e0d4 100644
>> --- a/net/openvswitch/conntrack.c
>> +++ b/net/openvswitch/conntrack.c
>> @@ -540,6 +540,16 @@ static int ovs_ct_add_helper(struct ovs_conntrack_info
>> *info, const char *name,
>> return 0;
>> }
>>
>> +static bool label_zero(const struct ovs_key_ct_labels *labels)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < sizeof(*labels); i++)
>> + if (labels->ct_labels[i])
>> + return false;
>> + return true;
>> +}
>> +
> There is already function called labels_nonzero(), This can be reused
> for labels check.
>
>
>> static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
>> [OVS_CT_ATTR_COMMIT] = { .minlen = 0, .maxlen = 0 },
>> [OVS_CT_ATTR_ZONE] = { .minlen = sizeof(u16),
>> @@ -589,6 +599,10 @@ static int parse_ct(const struct nlattr *attr, struct
>> ovs_conntrack_info *info,
>> case OVS_CT_ATTR_MARK: {
>> struct md_mark *mark = nla_data(a);
>>
>> + if (!mark->mask) {
>> + OVS_NLERR(log, "ct_mark mask cannot be 0");
>> + return -EINVAL;
>> + }
>> info->mark = *mark;
>> break;
>> }
>> @@ -597,6 +611,10 @@ static int parse_ct(const struct nlattr *attr, struct
>> ovs_conntrack_info *info,
>> case OVS_CT_ATTR_LABELS: {
>> struct md_labels *labels = nla_data(a);
>>
>> + if (label_zero(&labels->mask)) {
>> + OVS_NLERR(log, "ct_labels mask cannot be 0");
>> + return -EINVAL;
>> + }
>
> After this flow install stage sanity check for labels, there is no
> need for check in CONFIG_NF_CONNTRACK_LABELS action execution.
OK, thanks for the review. I'll fix these up.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
