netdev - RE: [PATCH]: tipc: Fix oops on send prior to entering networked mode

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]

Message-ID: <29C1DC0826876849BDD9F1C67ABA2943070F0D92@ala-mail09.corp.ad.wrs.com>
Date:	Tue, 23 Feb 2010 07:02:16 -0800
From:	"Stephens, Allan" <allan.stephens@...driver.com>
To:	"Neil Horman" <nhorman@...driver.com>
Cc:	<netdev@...r.kernel.org>, <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

Neil wrote:

> > 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:
> > 
> That seems like it might be a problem in and of itself.  If 
> the startup/shutdown code relies on the state of the 
> implementation, perhaps that worth cleaning up so as to avoid 
> a race condition.

I'm afraid I don't understand what you mean here.  It sounds like you're
saying that TIPC's code shouldn't rely on the state of its own
implementation.

> > 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.
> > 
> I'm happy to give this a try, but I'm a bit concerned by this 
> approach.  It certainly seems like it will solve the problem 
> at hand, but it doesn't seem to address the underlying cause, 
> which is that you can execute code paths which the state of 
> the implementation doesn't/shouldn't allow.  In other words, 
> this solve the immediate problem, but it still allows someone 
> to try send data via tipc before tipc is ready to send data.  
> It would be nice if we could deal with the larger problem.

I don't think you've stated the issue correctly.  The problem you
encountered isn't that TIPC is allowing users to send data before TIPC
is ready, it's that TIPC is allowing users to send data *off-node*
before TIPC is ready.  TIPC was deliberately designed so that messages
could be sent within the node itself as soon as possible, without having
to wait for full connectivity to the rest of the network, and this it
does quite well.  Later, once connectivity to the network is
established, TIPC allows applications to send data to other nodes, and
this also appears to work properly providing the applications use TIPC's
location transparent service naming form of addressing.

What we don't appear to have anticipated is that someone would attempt
to send messages to another node without first receiving an indication
from TIPC that the node could be reached.  As far as I can tell, the
only way that the crash you encountered could be generated would be if
your application blindly tries to send to a named service on a specified
node without first waiting for notification that the node exists.
(Please correct me if I'm wrong about this.)  While this operation is
certainly legal, including a specific node address in a send operation
kind of defeats the whole location transparent addressing premise on
which TIPC is based and I'm wondering if it's really necessary to do
things this way.  Regardless, the fix I've suggested seems to me to be a
reasonable way of blocking premature sends off-node while still
permitting on-node sends to work, and should make everyone happy.

> I'll let you know how the above patch goes.

Thanks,
Al 
--
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