[ovs-dev] [datapath minor fixes 2/3] datapath: avoid double free routing entry in vxlan_port xmit

[Jesse Gross]

I agree that the memory handling is too fragile here. Maybe we should
just make people attach the dst before calling into tunneling routines
though. It might be easier considering the multiple entry points (i.e.
GRE calls iptunnel_xmit() directly).

Several callers are definitely expecting iptunnel_xmit() to
potentially return an error code. It's probably less confusing to not
buck that trend and keep it as is. I guess in theory it could return
an error code, it just doesn't right now.

On Wed, Oct 1, 2014 at 1:11 PM, Andy Zhou <azhou at nicira.com> wrote:
> It turns out rout entry will not be attached to skb until
> iptunnel_xmit, which does not return negative value. So there is no
> bug here.
>
> I will drop this patch, and modify the upstream vport-geneve.c accordingly.
>
> It is pretty subtle that the correctness of free depends on the
> implementation details of the API.  May be it will be better for
> vxlan_xmit_skb always
> attach the route skb at the very beginning?
>
> Should we also make iptunnel_xmit() return type as unsigned int?
>
> On Wed, Oct 1, 2014 at 10:42 AM, Jesse Gross <jesse at nicira.com> wrote:
>> On Wed, Oct 1, 2014 at 1:02 AM, Andy Zhou <azhou at nicira.com> wrote:
>>> Route entry will be free on error by vport when freeing skb.
>>> additional error check and free after xmit() will cause double free.
>>>
>>> Signed-off-by: Andy Zhou <azhou at nicira.com>
>>
>> I'm not sure that the route entry is attached to the skb in all cases.
>> If it's not then, this patch will cause a leak in those situations.