[PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[Steffen Klassert]

git commit 97cac082 (ipv6: Store route neighbour in rt6_info struct)
added a neighbour pointer to rt6_info. Currently we don't initialize
this pointer at allocation time. We assume this pointer to be valid
if it is not a null pointer, so initialize it on allocation.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/route.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ceff71d..6cc6c88 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -273,7 +273,7 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
 					0, 0, flags);
 
 	if (rt) {
-		memset(&rt->rt6i_table, 0,
+		memset(&rt->n, 0,
 		       sizeof(*rt) - sizeof(struct dst_entry));
 		rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
 	}
-- 
1.7.0.4

Re: [PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[Eric Dumazet]

On Thu, 2012-07-05 at 15:18 +0200, Steffen Klassert wrote:
> git commit 97cac082 (ipv6: Store route neighbour in rt6_info struct)
> added a neighbour pointer to rt6_info. Currently we don't initialize
> this pointer at allocation time. We assume this pointer to be valid
> if it is not a null pointer, so initialize it on allocation.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/ipv6/route.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index ceff71d..6cc6c88 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -273,7 +273,7 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
>  					0, 0, flags);
>  
>  	if (rt) {
> -		memset(&rt->rt6i_table, 0,
> +		memset(&rt->n, 0,
>  		       sizeof(*rt) - sizeof(struct dst_entry));
>  		rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
>  	}

Hmm, could we find a way to avoid this for future changes ?

We know dst_entry is the first field, so maybe :

if (rt) {
	struct dst_entry *dst = (struct dst_entry *)rt;

	memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));

Re: [PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 5 Jul 2012 15:18:28 +0200

> git commit 97cac082 (ipv6: Store route neighbour in rt6_info struct)
> added a neighbour pointer to rt6_info. Currently we don't initialize
> this pointer at allocation time. We assume this pointer to be valid
> if it is not a null pointer, so initialize it on allocation.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied, but as Eric said we need to find a way to avoid having to
make changes like this every time we simply want to add a struct
member to rt6_info.

Re: [PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[Steffen Klassert]

On Thu, Jul 05, 2012 at 05:16:44PM +0200, Eric Dumazet wrote:
> On Thu, 2012-07-05 at 15:18 +0200, Steffen Klassert wrote:
> >  
> >  	if (rt) {
> > -		memset(&rt->rt6i_table, 0,
> > +		memset(&rt->n, 0,
> >  		       sizeof(*rt) - sizeof(struct dst_entry));
> >  		rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
> >  	}
> 
> Hmm, could we find a way to avoid this for future changes ?
> 
> We know dst_entry is the first field, so maybe :
> 
> if (rt) {
> 	struct dst_entry *dst = (struct dst_entry *)rt;
> 
> 	memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
> 

Yes, I think we need to do something like this.

I've just noticed that ip6_blackhole_route, xfrm_alloc_dst,
dn_route_output_slow and dn_route_input_slow have the same issue.

Re: [PATCH net-next] ipv6: Initialize the neighbour pointer of rt6_info on allocation

[Steffen Klassert]

On Fri, Jul 06, 2012 at 08:54:26AM +0200, Steffen Klassert wrote:
> On Thu, Jul 05, 2012 at 05:16:44PM +0200, Eric Dumazet wrote:
> > 
> > Hmm, could we find a way to avoid this for future changes ?
> > 
> > We know dst_entry is the first field, so maybe :
> > 
> > if (rt) {
> > 	struct dst_entry *dst = (struct dst_entry *)rt;
> > 
> > 	memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
> > 
> 
> Yes, I think we need to do something like this.
> 
> I've just noticed that ip6_blackhole_route, xfrm_alloc_dst,
> dn_route_output_slow and dn_route_input_slow have the same issue.

Actually, decnet is ok. So I'll send patches with the change you
suggested for ipv6 and xfrm.