lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 22 Feb 2010 14:44:48 -0800
From:	"Stephens, Allan" <allan.stephens@...driver.com>
To:	"Neil Horman" <nhorman@...driver.com>, <netdev@...r.kernel.org>
Cc:	<per.liden@...csson.com>, <jon.maloy@...csson.com>,
	<tipc-discussion@...ts.sourceforge.net>, <davem@...emloft.net>
Subject: RE: [PATCH]: tipc: Fix oops on send prior to entering networked mode

Hi Neil:

Good work on spotting this bug, and in tracking down the cause.  I took
a look at your patch, but there are a couple of problems I can see with
the approach you've taken to fix things:

1) The check you've added to send_msg() in net/tipc/socket.c will help
prevent things from blowing up if the message sender is using an AF_MIPC
socket from user space, but it won't help prevent a similar oops if an
"application" uses TIPC's native API to send a message directly from
kernel space.

2) The other change you made to defer setting tipc_mode to TIPC_NET_MODE
will cause problems if TIPC has to bail out during tipc_net_start()
because of problems.  Specifically, the ensuing call to tipc_net_stop()
won't get a chance to clean up any partial initialization that got done
prior to the initialization problem, which could result in memory leaks.

Fortunately, I think there's a relatively easy solution.  Since TIPC
always needs to call tipc_node_select() in order to send an off-node
message, you should be able to add the necessary error checking there.
I'm thinking of something along the lines of:

static inline struct tipc_node *tipc_node_select(u32 addr, u32 selector)
{
	if (likely(in_own_cluster(addr)))
		return tipc_local_nodes ?
tipc_local_nodes[tipc_node(addr)] : NULL;
	return tipc_net->zones ? tipc_node_select_next_hop(addr,
selector) : NULL;
}

Please give this a try and let us know how things go.

Regards,
Al

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@...driver.com] 
> Sent: Friday, February 19, 2010 2:41 PM
> To: netdev@...r.kernel.org
> Cc: per.liden@...csson.com; jon.maloy@...csson.com; Stephens, 
> Allan; tipc-discussion@...ts.sourceforge.net; 
> davem@...emloft.net; nhorman@...driver.com
> Subject: [PATCH]: tipc: Fix oops on send prior to entering 
> networked mode
> 
> Fix TIPC to disallow sending to remote addresses prior to 
> entering NET_MODE
> 
> user programs can oops the kernel by sending datagrams via 
> AF_TIPC prior to entering networked mode.  The following 
> backtrace has been observed:
> 
> ID: 13459  TASK: ffff810014640040  CPU: 0   COMMAND: "tipc-client"
> #0 [ffff81002d9a5810] crash_kexec at ffffffff800ac5b9
> #1 [ffff81002d9a58d0] __die at ffffffff80065127
> #2 [ffff81002d9a5910] do_page_fault at ffffffff80066da7
> #3 [ffff81002d9a5a00] error_exit at ffffffff8005dde9 
> [exception RIP: tipc_node_select_next_hop+90]
> RIP: ffffffff8869d3c3  RSP: ffff81002d9a5ab8  RFLAGS: 00010202
> RAX: 0000000000000001  RBX: 0000000000000001  RCX: 0000000000000001
> RDX: 0000000000000000  RSI: 0000000000000001  RDI: 0000000001001001
> RBP: 0000000001001001   R8: 0074736575716552   R9: 0000000000000000
> R10: ffff81003fbd0680  R11: 00000000000000c8  R12: 0000000000000008
> R13: 0000000000000001  R14: 0000000000000001  R15: ffff810015c6ca00
> ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
> #4 [ffff81002d9a5ab0] tipc_node_select_next_hop at ffffffff8869d3b1
> #5 [ffff81002d9a5ae0] tipc_link_send_sections_fast at ffffffff88698558
> #6 [ffff81002d9a5be0] tipc_forward2port at ffffffff8869eb1d
> #7 [ffff81002d9a5c10] tipc_send2port at ffffffff8869eb79
> #8 [ffff81002d9a5c30] send_msg at ffffffff886a1d0b
> #9 [ffff81002d9a5cb0] sock_sendmsg at ffffffff80055261
> RIP: 0000003cbd8d49a3  RSP: 00007fffc84e0be8  RFLAGS: 00010206
> RAX: 000000000000002c  RBX: ffffffff8005d116  RCX: 0000000000000000
> RDX: 0000000000000008  RSI: 00007fffc84e0c00  RDI: 0000000000000003
> RBP: 0000000000000000   R8: 00007fffc84e0c10   R9: 0000000000000010
> R10: 0000000000000000  R11: 0000000000000246  R12: 0000000000000000
> R13: 00007fffc84e0d10  R14: 0000000000000000  R15: 00007fffc84e0c30
> ORIG_RAX: 000000000000002c  CS: 0033  SS: 002b
> 
> What happens is that, when the tipc module in inserted it 
> enters a standalone node mode in which communication to its 
> own address is allowed <0.0.0> but not to other addresses, 
> since the appropriate data structures have not been allocated 
> yet (specifically the tipc_net pointer).  There is nothing 
> stopping a client from trying to send such a message however, 
> and if that happens, we attempt to dereference tipc_net.zones 
> while the pointer is still NULL, and explode.  The fix is to 
> add a check at the start of the send_msg path to ensure that 
> we've allocated the tipc_net pointers and entered networked 
> mode prior to allowing a send to any destination other than 
> our loopback address.
> 
> This patch has received minimal testing, but fixes the issue. 
>  Through reviews are appreciated.
> 
> Signed-off-by: Neil Horman <nhorman@...driver.com>
> 
> CC: Per Liden <per.liden@...csson.com>
> CC: Jon Maloy <jon.maloy@...csson.com>
> CC: Allan Stephens <allan.stephens@...driver.com>
> CC: David S. Miller <davem@...emloft.net>
> CC: Neil Horman <nhorman@...driver.com>
> CC: tipc-discussion@...ts.sourceforge.net
> 
> 
>  net.c    |    2 +-
>  socket.c |    4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/tipc/net.c b/net/tipc/net.c index 
> 7906608..512b33c 100644
> --- a/net/tipc/net.c
> +++ b/net/tipc/net.c
> @@ -278,7 +278,6 @@ int tipc_net_start(u32 addr)
>  	tipc_cfg_stop();
>  
>  	tipc_own_addr = addr;
> -	tipc_mode = TIPC_NET_MODE;
>  	tipc_named_reinit();
>  	tipc_port_reinit();
>  
> @@ -289,6 +288,7 @@ int tipc_net_start(u32 addr)
>  		return res;
>  	}
>  
> +	tipc_mode = TIPC_NET_MODE;
>  	tipc_k_signal((Handler)tipc_subscr_start, 0);
>  	tipc_k_signal((Handler)tipc_cfg_init, 0);
>  
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 
> 1ea64f0..45229fd 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -526,6 +526,10 @@ static int send_msg(struct kiocb *iocb, 
> struct socket *sock,
>  	if (iocb)
>  		lock_sock(sk);
>  
> +	if ((tipc_mode != TIPC_NET_MODE) &&
> +	    (dest->addr.name.domain != tipc_own_addr))
> +		return -EHOSTUNREACH;
> +
>  	needs_conn = (sock->state != SS_READY);
>  	if (unlikely(needs_conn)) {
>  		if (sock->state == SS_LISTENING) {
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ