Re: [PATCH net] openvswitch: Zero flows on allocation.

[David Miller]

From: Jesse Gross 
Date: Fri, 18 Sep 2015 19:06:14 -0700

> @@ -80,7 +80,7 @@ struct sw_flow *ovs_flow_alloc(void)
>   struct flow_stats *stats;
>   int node;
>  
> - flow = kmem_cache_alloc(flow_cache, GFP_KERNEL);
> + flow = kmem_cache_alloc(flow_cache, GFP_KERNEL | __GFP_ZERO);
>   if (!flow)

Like Eric, I prefer that you use kmem_cache_zalloc() to fix
this.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] openvswitch: Zero flows on allocation.

[Jesse Gross]

On Sun, Sep 20, 2015 at 11:24 PM, David Miller  wrote:
> From: Jesse Gross 
> Date: Fri, 18 Sep 2015 19:06:14 -0700
>
>> @@ -80,7 +80,7 @@ struct sw_flow *ovs_flow_alloc(void)
>>   struct flow_stats *stats;
>>   int node;
>>
>> - flow = kmem_cache_alloc(flow_cache, GFP_KERNEL);
>> + flow = kmem_cache_alloc(flow_cache, GFP_KERNEL | __GFP_ZERO);
>>   if (!flow)
>
> Like Eric, I prefer that you use kmem_cache_zalloc() to fix
> this.

Sure, I'll make both changes and send out a new patch in a bit.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] openvswitch: Zero flows on allocation.

[Eric Dumazet]

On Fri, 2015-09-18 at 19:06 -0700, Jesse Gross wrote:

> 
> Fixes: 07148121 ("openvswitch: Eliminate memset() from flow_extract.")
> Signed-off-by: Jesse Gross 
> ---
>  net/openvswitch/flow_table.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index d22d8e9..5248322 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -80,7 +80,7 @@ struct sw_flow *ovs_flow_alloc(void)
>   struct flow_stats *stats;
>   int node;
>  
> - flow = kmem_cache_alloc(flow_cache, GFP_KERNEL);
> + flow = kmem_cache_alloc(flow_cache, GFP_KERNEL | __GFP_ZERO);

Or kmem_cache_zalloc(flow_cache, GFP_KERNEL) ?




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net] openvswitch: Zero flows on allocation.

[Pravin Shelar]

On Fri, Sep 18, 2015 at 7:06 PM, Jesse Gross  wrote:
> OVS tries to be clever about not touching the parts of a flow that
> aren't used. This can include leaving pieces of memory uninitialized
> if the mask is zero and therefore the value would be ignored anyways.
>
> While this works fine for the purposes of matching (which must always
> look at the mask), serialization to netlink can be problematic. Since
> the flow and the mask are serialized separately, the uninitialized
> portions of the flow can be encoded with whatever values happen to be
> present.
>
> In terms of functionality, this has little effect since these fields
> will be masked out by definition. However, it leaks kernel memory to
> userspace, which is a potential security vulnerability.
>
> This zeros the flows as they are allocated and installed. This was
> always intended to be the case as the memory optimizations were only
> supposed to apply to per-packet flow operations.
>
Good catch.

> Fixes: 07148121 ("openvswitch: Eliminate memset() from flow_extract.")
> Signed-off-by: Jesse Gross 
> ---
>  net/openvswitch/flow_table.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index d22d8e9..5248322 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -80,7 +80,7 @@ struct sw_flow *ovs_flow_alloc(void)
> struct flow_stats *stats;
> int node;
>
> -   flow = kmem_cache_alloc(flow_cache, GFP_KERNEL);
> +   flow = kmem_cache_alloc(flow_cache, GFP_KERNEL | __GFP_ZERO);
> if (!flow)
> return ERR_PTR(-ENOMEM);

There are few flow members set to zero that are unnecessary after this change.

>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html