On Fri, Jul 8, 2016 at 1:44 PM, Jesse Gross <je...@kernel.org> wrote:
> On Thu, Jul 7, 2016 at 5:23 PM, Pravin B Shelar <pshe...@ovn.org> wrote:
>> diff --git a/datapath/linux/compat/include/net/dst_metadata.h
>> b/datapath/linux/compat/include/net/dst_metadata.h
>> index f15bb03..b54cfc0 100644
>> --- a/datapath/linux/compat/include/net/dst_metadata.h
>> +++ b/datapath/linux/compat/include/net/dst_metadata.h
>> +static inline void ovs_tun_rx_dst(struct ip_tunnel_info *info, int md_size)
>> +{
>> + /* No need to allocate for OVS backport case. */
>> +#if 0
>> + struct metadata_dst *tun_dst;
>> + struct ip_tunnel_info *info;
>> +
>> + tun_dst = metadata_dst_alloc(md_size, GFP_ATOMIC);
>> + if (!tun_dst)
>> + return NULL;
>> +#endif
>> + info->mode = 0;
>> + info->options_len = 0;
>> +}
>
> One thing that I noticed when reviewing the last patch is that
> upstream does a memset() (through __metadata_dst_init()) of
> ip_tunnel_info as part of this when metadata_dst_alloc() is called -
> which is missing here since the storage space is from the stack. This
> also applies to the allocation of the metadata_dst when it is
> allocated as part of the flow.
>
> I don't think that this has any effect in practice at the moment since
> we current overwrite the whole thing whenever we use the metadata_dst.
> However, it would probably be good to match upstream to avoid any
> subtle bugs from uninitialized memory.
I added memset to metadata_dst_alloc(). I will post separate patch to
add memset to ovs_tun_rx_dst() since it requires parameter change, it
need more changes all call sites.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
