[ovs-dev] Prevent kernel warning length of ovs attribute.

Linux kernel commit 6e237d099fac introduced warnings when validating the
length of some types. This exposed a bug in dpif-netlink.c
dpif_netlink_vport_to_ofpbuf where ovs was passing the upcall pids as
variable length array while the kernel was expecting a single u32. This
resulted in the kernel reporting "netlink: 'ovs-vswitchd': attribute
type 5 has an invalid length.".

This patch changes the kernel to expect NLA_UNSPEC, which is for
arbitrary length data.

Signed-off-by: Nathanael Davison <nathanael.davison@citrix.com>
---
 datapath/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Wed, Dec 12, 2018 at 03:06:43PM +0000, Nathanael Davison wrote:
> Linux kernel commit 6e237d099fac introduced warnings when validating the
> length of some types. This exposed a bug in dpif-netlink.c
> dpif_netlink_vport_to_ofpbuf where ovs was passing the upcall pids as
> variable length array while the kernel was expecting a single u32. This
> resulted in the kernel reporting "netlink: 'ovs-vswitchd': attribute
> type 5 has an invalid length.".
> 
> This patch changes the kernel to expect NLA_UNSPEC, which is for
> arbitrary length data.
> 
> Signed-off-by: Nathanael Davison <nathanael.davison@citrix.com>
> ---
>  datapath/datapath.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index e3d3c8c..3204bf3 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -2170,7 +2170,7 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
>  	[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
>  	[OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
>  	[OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
> -	[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_U32 },
> +	[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_UNSPEC },
>  	[OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
>  };

This seems like it fell through the cracks.  Sorry about that.

Nathanael, the usual way to get fixes into the OVS datapath directory is
to first get them into the upstream kernel "net-next".  It doesn't look
like this is there yet, so it's not yet time to submit it to ovs-dev.
Instead, you should start with the Linux netdev mailing list (but you'll
need to modify the patch to apply to net/openvswitch/datapath.c in that
tree instead).

Greg (CCed) might be able to help, if you need it.

Thanks,

Ben.

Thanks for getting back to me, I'll look into getting it into the 
upstream kernel.

Nathanael

On 22/02/2019 23:50, Ben Pfaff wrote:
> On Wed, Dec 12, 2018 at 03:06:43PM +0000, Nathanael Davison wrote:
>> Linux kernel commit 6e237d099fac introduced warnings when validating the
>> length of some types. This exposed a bug in dpif-netlink.c
>> dpif_netlink_vport_to_ofpbuf where ovs was passing the upcall pids as
>> variable length array while the kernel was expecting a single u32. This
>> resulted in the kernel reporting "netlink: 'ovs-vswitchd': attribute
>> type 5 has an invalid length.".
>>
>> This patch changes the kernel to expect NLA_UNSPEC, which is for
>> arbitrary length data.
>>
>> Signed-off-by: Nathanael Davison <nathanael.davison@citrix.com>
>> ---
>>   datapath/datapath.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index e3d3c8c..3204bf3 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -2170,7 +2170,7 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
>>   	[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
>>   	[OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
>>   	[OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
>> -	[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_U32 },
>> +	[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_UNSPEC },
>>   	[OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
>>   };
> This seems like it fell through the cracks.  Sorry about that.
>
> Nathanael, the usual way to get fixes into the OVS datapath directory is
> to first get them into the upstream kernel "net-next".  It doesn't look
> like this is there yet, so it's not yet time to submit it to ovs-dev.
> Instead, you should start with the Linux netdev mailing list (but you'll
> need to modify the patch to apply to net/openvswitch/datapath.c in that
> tree instead).
>
> Greg (CCed) might be able to help, if you need it.
>
> Thanks,
>
> Ben.