>From 06f7ebb1dade2f0dbf872ea2bedf17cff4734bdd Mon Sep 17 00:00:00 2001 From: Olaf Kirch <okir@xxxxxxx> Date: Thu, 6 Aug 2015 16:27:20 +0200 Subject: [PATCH] Fix memory corruption in PMAP_CALLIT code We have seen occasional reports of a commercial security scanner triggering crashes in rpcbind, but these were fairly elusive. Now we finally got a usable core file showing that rpcbind crashed in svc_dodestroy when trying to free a corrupted xprt->xp_netid pointer. Closer inspection suggested that the pointer variable actually contained a sockaddr_in. Here's how I think the memory corruption happens: - A PMAP_CALLIT call comes in on IPv4 UDP - rpcbind duplicates the caller's address to a netbuf and stores it in FINFO[0].caller_addr. caller_addr->buf now points to a memory region A with a size of 16 bytes - rpcbind forwards the call to the local service, receives a reply - when processing the reply, it does this in xprt_set_caller: xprt->xp_rtaddr = *FINFO[0].caller_addr where xprt is the UDP transport on which it received the PMAP_CALLIT request. It sends out the reply, and then frees the netbuf caller_addr and caller_addr.buf. However, it does not clear xp_rtaddr, so xp_rtaddr.buf now refers to memory region A, which is free. - When the next call comes in on the UDP/IPv4 socket, svc_dg_recv will be called, which will set xp_rtaddr to the client's address. It will reuse the buffer inside xp_rtaddr, ie it will write a sockaddr_in to region A. So, this explains how memory gets corrupted. Here's why that eventually lead to a crash in svc_dodestroy. Some time down the road, an incoming TCP connection is accepted, allocating a fresh SVCXPRT. The memory region A is inside the new SVCXPRT - While processing the TCP call, another UDP call comes in, again overwriting region A with the client's address - TCP client closes connection. In svc_destroy, we now trip over the garbage left in region A The fix seems to be to make xprt_set_caller copy the caller's address to xprt->xp_rtaddr using __rpc_set_netbuf rather than just overwrite the netbuf. Signed-off-by: Olaf Kirch <okir@xxxxxxx> --- src/rpcb_svc_com.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c index ff9ce6b..79fb3f5 100644 --- a/src/rpcb_svc_com.c +++ b/src/rpcb_svc_com.c @@ -1186,9 +1186,10 @@ check_rmtcalls(struct pollfd *pfds, int nfds) static void xprt_set_caller(SVCXPRT *xprt, struct finfo *fi) { + const struct netbuf *caller = fi->caller_addr; u_int32_t *xidp; - *(svc_getrpccaller(xprt)) = *(fi->caller_addr); + __rpc_set_netbuf(svc_getrpccaller(xprt), caller->buf, caller- >len); xidp = __rpcb_get_dg_xidp(xprt); *xidp = fi->caller_xid; } -- 1.7.12.4 -- It is better to keep your mouth closed and let people think you are a fool than to open it and remove all doubt. -- Mark Twain -------------------------------------------- Olaf Kirch - Director SUSE Linux Enterprise; R&D (okir@xxxxxxxx) SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html