Commit d0021b252eaf65ca07ed14f0d66425dd9ccab9a6

Authored by Neil Horman
Committed by David S. Miller
1 parent 6d55cb91a0

tipc: Fix oops on send prior to entering networked mode (v3)

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"
[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
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 pretty straightforward.  Since these oopses all arise from
the dereference of global pointers prior to their assignment to allocated
values, and since these allocations are small (about 2k total), lets convert
these pointers to static arrays of the appropriate size.  All the accesses to
these bits consider 0/NULL to be a non match when searching, so all the lookups
still work properly, and there is no longer a chance of a bad dererence
anywhere.  As a bonus, this lets us eliminate the setup/teardown routines for
those pointers, and elimnates the need to preform any locking around them to
prevent access while their being allocated/freed.

I've updated the tipc_net structure to behave this way to fix the exact reported
problem, and also fixed up the tipc_bearers and media_list arrays to fix an
obvious simmilar problem that arises from issuing tipc-config commands to
manipulate bearers/links prior to entering networked mode

I've tested this for a few hours by running the sanity tests and stress test
with the tipcutils suite, and nothing has fallen over.  There have been a few
lockdep warnings, but those were there before, and can be addressed later, as
they didn't actually result in any deadlock.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Allan Stephens <allan.stephens@windriver.com>
CC: David S. Miller <davem@davemloft.net>
CC: tipc-discussion@lists.sourceforge.net

 bearer.c |   37 ++++++-------------------------------
 bearer.h |    2 +-
 net.c    |   25 ++++---------------------
 3 files changed, 11 insertions(+), 53 deletions(-)
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 3 changed files with 11 additions and 53 deletions Side-by-side Diff

... ... @@ -45,10 +45,10 @@
45 45  
46 46 #define MAX_ADDR_STR 32
47 47  
48   -static struct media *media_list = NULL;
  48 +static struct media media_list[MAX_MEDIA];
49 49 static u32 media_count = 0;
50 50  
51   -struct bearer *tipc_bearers = NULL;
  51 +struct bearer tipc_bearers[MAX_BEARERS];
52 52  
53 53 /**
54 54 * media_name_valid - validate media name
55 55  
... ... @@ -108,9 +108,11 @@
108 108 int res = -EINVAL;
109 109  
110 110 write_lock_bh(&tipc_net_lock);
111   - if (!media_list)
112   - goto exit;
113 111  
  112 + if (tipc_mode != TIPC_NET_MODE) {
  113 + warn("Media <%s> rejected, not in networked mode yet\n", name);
  114 + goto exit;
  115 + }
114 116 if (!media_name_valid(name)) {
115 117 warn("Media <%s> rejected, illegal name\n", name);
116 118 goto exit;
117 119  
... ... @@ -660,33 +662,10 @@
660 662  
661 663  
662 664  
663   -int tipc_bearer_init(void)
664   -{
665   - int res;
666   -
667   - write_lock_bh(&tipc_net_lock);
668   - tipc_bearers = kcalloc(MAX_BEARERS, sizeof(struct bearer), GFP_ATOMIC);
669   - media_list = kcalloc(MAX_MEDIA, sizeof(struct media), GFP_ATOMIC);
670   - if (tipc_bearers && media_list) {
671   - res = 0;
672   - } else {
673   - kfree(tipc_bearers);
674   - kfree(media_list);
675   - tipc_bearers = NULL;
676   - media_list = NULL;
677   - res = -ENOMEM;
678   - }
679   - write_unlock_bh(&tipc_net_lock);
680   - return res;
681   -}
682   -
683 665 void tipc_bearer_stop(void)
684 666 {
685 667 u32 i;
686 668  
687   - if (!tipc_bearers)
688   - return;
689   -
690 669 for (i = 0; i < MAX_BEARERS; i++) {
691 670 if (tipc_bearers[i].active)
692 671 tipc_bearers[i].publ.blocked = 1;
... ... @@ -695,10 +674,6 @@
695 674 if (tipc_bearers[i].active)
696 675 bearer_disable(tipc_bearers[i].publ.name);
697 676 }
698   - kfree(tipc_bearers);
699   - kfree(media_list);
700   - tipc_bearers = NULL;
701   - media_list = NULL;
702 677 media_count = 0;
703 678 }
... ... @@ -114,7 +114,7 @@
114 114  
115 115 struct link;
116 116  
117   -extern struct bearer *tipc_bearers;
  117 +extern struct bearer tipc_bearers[];
118 118  
119 119 void tipc_media_addr_printf(struct print_buf *pb, struct tipc_media_addr *a);
120 120 struct sk_buff *tipc_media_get_names(void);
... ... @@ -116,7 +116,8 @@
116 116 */
117 117  
118 118 DEFINE_RWLOCK(tipc_net_lock);
119   -struct network tipc_net = { NULL };
  119 +struct _zone *tipc_zones[256] = { NULL, };
  120 +struct network tipc_net = { tipc_zones };
120 121  
121 122 struct tipc_node *tipc_net_select_remote_node(u32 addr, u32 ref)
122 123 {
123 124  
124 125  
... ... @@ -158,28 +159,12 @@
158 159 }
159 160 }
160 161  
161   -static int net_init(void)
162   -{
163   - memset(&tipc_net, 0, sizeof(tipc_net));
164   - tipc_net.zones = kcalloc(tipc_max_zones + 1, sizeof(struct _zone *), GFP_ATOMIC);
165   - if (!tipc_net.zones) {
166   - return -ENOMEM;
167   - }
168   - return 0;
169   -}
170   -
171 162 static void net_stop(void)
172 163 {
173 164 u32 z_num;
174 165  
175   - if (!tipc_net.zones)
176   - return;
177   -
178   - for (z_num = 1; z_num <= tipc_max_zones; z_num++) {
  166 + for (z_num = 1; z_num <= tipc_max_zones; z_num++)
179 167 tipc_zone_delete(tipc_net.zones[z_num]);
180   - }
181   - kfree(tipc_net.zones);
182   - tipc_net.zones = NULL;
183 168 }
184 169  
185 170 static void net_route_named_msg(struct sk_buff *buf)
... ... @@ -282,9 +267,7 @@
282 267 tipc_named_reinit();
283 268 tipc_port_reinit();
284 269  
285   - if ((res = tipc_bearer_init()) ||
286   - (res = net_init()) ||
287   - (res = tipc_cltr_init()) ||
  270 + if ((res = tipc_cltr_init()) ||
288 271 (res = tipc_bclink_init())) {
289 272 return res;
290 273 }