[ovs-dev] [PATCH] datapath-windows: Fixed buffer overflow in OvsInitVportWithNicParam

[Nithin Raju]

Thanks for digging this up.

-----Original Message-----
From: Paul Boca <pboca at cloudbasesolutions.com>
Date: Thursday, April 21, 2016 at 11:58 PM
To: Nithin Raju <nithin at vmware.com>, "dev at openvswitch.org"
<dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Fixed buffer overflow in
OvsInitVportWithNicParam

>Thanks for review!
>
>You're right Nithin, there's no reason to extend to 32 bytes.
>I've searched about why Microsoft added that size to MAC address and
>found in 
>NDIS_FILTER_ATTACH_PARAMETERS that there is a field MacAddressLength
>specific to 
>the type media, but for NdisMedium802_3 it is indeed 6 bytes.
>Will resubmit a V2 patch with this updated.
>
>Paul
>
>-----Original Message-----
>From: Nithin Raju [mailto:nithin at vmware.com]
>Sent: Friday, April 22, 2016 6:05 AM
>To: Paul Boca; dev at openvswitch.org
>Subject: Re: [ovs-dev] [PATCH] datapath-windows: Fixed buffer overflow in
>OvsInitVportWithNicParam
>
>Why do we need ethernet address to be macAddress to be 32 bytes?
>
>We should just make sure that when we copy, we use sizeof (<destination>)
>and in this case, it is sizeof (vport->macAddress).
>
>-- Nithin
>
>-----Original Message-----
>From: dev <dev-bounces at openvswitch.org> on behalf of Paul Boca
><pboca at cloudbasesolutions.com>
>Date: Monday, April 18, 2016 at 12:52 AM
>To: "dev at openvswitch.org" <dev at openvswitch.org>
>Subject: [ovs-dev] [PATCH] datapath-windows: Fixed buffer overflow in
>OvsInitVportWithNicParam
>
>>nicParam->PermanentMacAddress is 32 bytes and vport->permMacAddress is 6
>>bytes
>>
>>Signed-off-by: Paul-Daniel Boca <pboca at cloudbasesolutions.com>
>>---
>> datapath-windows/ovsext/DpInternal.h | 6 +++---
>> datapath-windows/ovsext/Vport.h      | 6 +++---
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/DpInternal.h
>>b/datapath-windows/ovsext/DpInternal.h
>>index a3ce311..760552d 100644
>>--- a/datapath-windows/ovsext/DpInternal.h
>>+++ b/datapath-windows/ovsext/DpInternal.h
>>@@ -41,9 +41,9 @@ typedef struct _OVS_VPORT_GET {
>> typedef struct _OVS_VPORT_EXT_INFO {
>>     uint32_t dpNo;
>>     uint32_t portNo;
>>-    uint8_t macAddress[ETH_ADDR_LEN];
>>-    uint8_t permMACAddress[ETH_ADDR_LEN];
>>-    uint8_t vmMACAddress[ETH_ADDR_LEN];
>>+    uint8_t macAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>>+    uint8_t permMACAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>>+    uint8_t vmMACAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>>     uint16_t nicIndex;
>>     uint32_t portId;
>>     uint32_t type;
>>diff --git a/datapath-windows/ovsext/Vport.h
>>b/datapath-windows/ovsext/Vport.h
>>index 373896d..3f18eb1 100644
>>--- a/datapath-windows/ovsext/Vport.h
>>+++ b/datapath-windows/ovsext/Vport.h
>>@@ -102,9 +102,9 @@ typedef struct _OVS_VPORT_ENTRY {
>>     NDIS_SWITCH_NIC_STATE  nicState;
>>     NDIS_SWITCH_PORT_TYPE  portType;
>> 
>>-    UINT8                  permMacAddress[ETH_ADDR_LEN];
>>-    UINT8                  currMacAddress[ETH_ADDR_LEN];
>>-    UINT8                  vmMacAddress[ETH_ADDR_LEN];
>>+    UINT8                  permMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>>+    UINT8                  currMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>>+    UINT8                  vmMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
>> 
>>     NDIS_SWITCH_PORT_NAME  hvPortName;
>>     IF_COUNTED_STRING      portFriendlyName;
>>-- 
>>2.7.2.windows.1
>>_______________________________________________
>>dev mailing list
>>dev at openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm
>>a
>>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=p
>>N
>>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=1bLNUgQWl_iQvCaPGr2m9s8F4v8L2
>>f
>>Bug6h3DWQopKA&s=CEIFPbcaXRdYRSic-iezwcrlPOBFvYcceYdwuN3xIQ8&e=
>