[ovs-dev] [PATCH v2] dpif: Fix dp_extra_info leak by reworking the allocation scheme.

[Ilya Maximets]

On 27.01.2020 16:04, Finn, Emma wrote:
> 
> 
>> -----Original Message-----
>> From: Stokes, Ian <ian.stokes at intel.com>
>> Sent: Monday 27 January 2020 14:46
>> To: Ilya Maximets <i.maximets at ovn.org>; ovs-dev at openvswitch.org; Ben
>> Pfaff <blp at ovn.org>
>> Cc: Damijan Skvarc <damjan.skvarc at gmail.com>; Finn, Emma
>> <emma.finn at intel.com>
>> Subject: Re: [PATCH v2] dpif: Fix dp_extra_info leak by reworking the
>> allocation scheme.
>>
>>
>>
>> On 1/23/2020 6:10 PM, Ilya Maximets wrote:
>>> dpctl module leaks the 'dp_extra_info' in case the dumped flow doesn't
>>> fit the dump filter while executing dpctl/dump-flows and also while
>>> executing dpctl/get-flow.
>>>
>>> This is already a 3rd attempt to fix all the leaks and incorrect usage
>>> of this string that definitely indicates poor initial design of the
>>> feature.
>>>
>>> Flow dump/get documentation clearly states that the caller does not
>>> own the data provided in dpif_flow.  Datapath still owns all the data
>>> and promises to not free/modify it until the next quiescent period,
>>> however we're requesting the caller to free 'dp_extra_info' and this
>>> obviously breaks the rules.
>>>
>>> This patch fixes the issue by by storing 'dp_extra_info' within
>>> 'struct dp_netdev_flow' making datapath to own it.  'dp_netdev_flow'
>>> is RCU-protected, so it will be valid until the next quiescent period.
>>>
>>> CC: Emma Finn <emma.finn at intel.com>
>>> Fixes: 0e8f5c6a38d0 ("dpif-netdev: Modified ovs-appctl
>>> dpctl/dump-flows command")
>>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>>> ---
>>
>> Thanks for the patch Ilya, appreciate the effort to fix this.
>>
>> I've only looked at the code and compile tested seems ok to me, I I believe
>> Emma is reviewing and testing also.
>>
>> Regards
>> Ian
>>
> Yes reviewed and tested. Thanks Ilya.
> 
> Tested-by: Emma Finn <emma.finn at intel.com>
> Acked-by: Emma Finn <emma.finn at intel.com>

Thanks!  Applied to master and branch-2.13.

Best regards, Ilya Maximets.