[PATCH]: net: ipv6: fix oops in inet_putpeer()

[PATCH]: net: ipv6: fix oops in inet_putpeer()

[Patrick McHardy]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 259 bytes --]

The attached patch fixes an oops in inet_putpeer(). Please see the 
changelog entry for details.

An alternative fix would be to check whether rt6_peer_ptr() returns
NULL before invoking inet_putpeer(), but properly initializing the
peer looks cleaner to me.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3608 bytes --]

commit 9bf0a619538e6667676cdcfd91018da9397c6fce
Author: Patrick McHardy <kaber@trash.net>
Date:   Sat Aug 18 17:17:30 2012 +0200

    net: ipv6: fix oops in inet_putpeer()
    
    Commit 97bab73f (inet: Hide route peer accesses behind helpers.) introduced
    a bug in xfrm6_policy_destroy(). The xfrm_dst's _rt6i_peer member is not
    initialized, causing a false positive result from inetpeer_ptr_is_peer(),
    which in turn causes a NULL pointer dereference in inet_putpeer().
    
    Pid: 314, comm: kworker/0:1 Not tainted 3.6.0-rc1+ #17 To Be Filled By O.E.M. To Be Filled By O.E.M./P4S800D-X
    EIP: 0060:[<c03abf93>] EFLAGS: 00010246 CPU: 0
    EIP is at inet_putpeer+0xe/0x16
    EAX: 00000000 EBX: f3481700 ECX: 00000000 EDX: 000dd641
    ESI: f3481700 EDI: c05e949c EBP: f551def4 ESP: f551def4
     DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
    CR0: 8005003b CR2: 00000070 CR3: 3243d000 CR4: 00000750
    DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
    DR6: ffff0ff0 DR7: 00000400
     f551df04 c0423de1 00000000 f3481700 f551df18 c038d5f7 f254b9f8 f551df28
     f34f85d8 f551df20 c03ef48d f551df3c c0396870 f30697e8 f24e1738 c05e98f4
     f5509540 c05cd2b4 f551df7c c0142d2b c043feb5 f5509540 00000000 c05cd2e8
     [<c0423de1>] xfrm6_dst_destroy+0x42/0xdb
     [<c038d5f7>] dst_destroy+0x1d/0xa4
     [<c03ef48d>] xfrm_bundle_flo_delete+0x2b/0x36
     [<c0396870>] flow_cache_gc_task+0x85/0x9f
     [<c0142d2b>] process_one_work+0x122/0x441
     [<c043feb5>] ? apic_timer_interrupt+0x31/0x38
     [<c03967eb>] ? flow_cache_new_hashrnd+0x2b/0x2b
     [<c0143e2d>] worker_thread+0x113/0x3cc
    
    Fix by adding a init_dst() callback to struct xfrm_policy_afinfo to
    properly initialize the dst's peer pointer.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 62b619e..976a81a 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -292,6 +292,8 @@ struct xfrm_policy_afinfo {
 						  struct flowi *fl,
 						  int reverse);
 	int			(*get_tos)(const struct flowi *fl);
+	void			(*init_dst)(struct net *net,
+					    struct xfrm_dst *dst);
 	int			(*init_path)(struct xfrm_dst *path,
 					     struct dst_entry *dst,
 					     int nfheader_len);
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index ef39812..f8c4c08 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -73,6 +73,13 @@ static int xfrm6_get_tos(const struct flowi *fl)
 	return 0;
 }
 
+static void xfrm6_init_dst(struct net *net, struct xfrm_dst *xdst)
+{
+	struct rt6_info *rt = (struct rt6_info *)xdst;
+
+	rt6_init_peer(rt, net->ipv6.peers);
+}
+
 static int xfrm6_init_path(struct xfrm_dst *path, struct dst_entry *dst,
 			   int nfheader_len)
 {
@@ -286,6 +293,7 @@ static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
 	.get_saddr = 		xfrm6_get_saddr,
 	.decode_session =	_decode_session6,
 	.get_tos =		xfrm6_get_tos,
+	.init_dst =		xfrm6_init_dst,
 	.init_path =		xfrm6_init_path,
 	.fill_dst =		xfrm6_fill_dst,
 	.blackhole_route =	ip6_blackhole_route,
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index c5a5165..5a2aa17 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1357,6 +1357,8 @@ static inline struct xfrm_dst *xfrm_alloc_dst(struct net *net, int family)
 
 		memset(dst + 1, 0, sizeof(*xdst) - sizeof(*dst));
 		xdst->flo.ops = &xfrm_bundle_fc_ops;
+		if (afinfo->init_dst)
+			afinfo->init_dst(net, xdst);
 	} else
 		xdst = ERR_PTR(-ENOBUFS);
 

Re: [PATCH]: net: ipv6: fix oops in inet_putpeer()

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 18 Aug 2012 17:32:31 +0200 (MEST)

> The attached patch fixes an oops in inet_putpeer(). Please see the
> changelog entry for details.
> 
> An alternative fix would be to check whether rt6_peer_ptr() returns
> NULL before invoking inet_putpeer(), but properly initializing the
> peer looks cleaner to me.

This is fine for now, applied, thanks Patrick.

There was a built-in assumption that xfrm_fill_dst() would run to
completion before we'd try to release these things, because there in
xfrm6_policy.c's xfrm6_fill_dst, we'd do the rt6_transfer_peer().

But that's obviously not the case if errors happen meanwhile.