tipc: fix broadcast link synchronization problem

In commit 790a53b23e88 ("tipc: extend broadcast link initialization
criteria") we tried to fix a problem with the initial synchronization
of broadcast link acknowledge values. Unfortunately that solution is
not sufficient to solve the issue.

We have seen it happen that LINK_PROTOCOL/STATE packets with a valid
non-zero unicast acknowledge number may bypass BCAST_PROTOCOL
initialization, NAME_DISTRIBUTOR and other STATE packets with invalid
broadcast acknowledge numbers, leading to premature opening of the
broadcast link. When the bypassed packets finally arrive, they are
inadvertently accepted, and the already correctly initialized
acknowledge number in the broadcast receive link is overwritten by
the invalid (zero) value of the said packets. After this the broadcast
link goes stale.

We now fix this by marking the packets where we know the acknowledge
value is or may be invalid, and then ignoring the acks from those.

To this purpose, we claim an unused bit in the header to indicate that
the value is invalid. We set the bit to 1 in the initial BCAST_PROTOCOL
synchronization packet and all initial ("bulk") NAME_DISTRIBUTOR
packets, plus those LINK_PROTOCOL packets sent out before the broadcast
links are fully synchronized.

This minor protocol update is fully backwards compatible.

Reported-by: John Thompson <thompa.atl@gmail.com>
Tested-by: John Thompson <thompa.atl@gmail.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 753f774cb46f39a7280515ca64c8c41897d9a480..aa1babbea385348f1ecd4b7467079fc442653094 100644 (file)
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -247,11 +247,17 @@ int tipc_bcast_rcv(struct net *net, struct tipc_link *l, struct sk_buff *skb)
  *
  * RCU is locked, no other locks set
  */
-void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, u32 acked)
+void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l,
+                       struct tipc_msg *hdr)
 {
        struct sk_buff_head *inputq = &tipc_bc_base(net)->inputq;
+       u16 acked = msg_bcast_ack(hdr);
        struct sk_buff_head xmitq;
 
+       /* Ignore bc acks sent by peer before bcast synch point was received */
+       if (msg_bc_ack_invalid(hdr))
+               return;
+
        __skb_queue_head_init(&xmitq);
 
        tipc_bcast_lock(net);
@@ -279,11 +285,11 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l,
        __skb_queue_head_init(&xmitq);
 
        tipc_bcast_lock(net);
-       if (msg_type(hdr) == STATE_MSG) {
+       if (msg_type(hdr) != STATE_MSG) {
+               tipc_link_bc_init_rcv(l, hdr);
+       } else if (!msg_bc_ack_invalid(hdr)) {
                tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), &xmitq);
                rc = tipc_link_bc_sync_rcv(l, hdr, &xmitq);
-       } else {
-               tipc_link_bc_init_rcv(l, hdr);
        }
        tipc_bcast_unlock(net);
 

diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h
index 5ffe34472ccd091d19e92137c379191b0d596844..855d53c64ab347ec5b37dc06a39e10748f7bf388 100644 (file)
--- a/net/tipc/bcast.h
+++ b/net/tipc/bcast.h
@@ -55,7 +55,8 @@ void tipc_bcast_dec_bearer_dst_cnt(struct net *net, int bearer_id);
 int  tipc_bcast_get_mtu(struct net *net);
 int tipc_bcast_xmit(struct net *net, struct sk_buff_head *list);
 int tipc_bcast_rcv(struct net *net, struct tipc_link *l, struct sk_buff *skb);
-void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, u32 acked);
+void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l,
+                       struct tipc_msg *hdr);
 int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l,
                        struct tipc_msg *hdr);
 int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg);

diff --git a/net/tipc/link.c b/net/tipc/link.c
index b36e16cdc945230f0460f46ef5c4fb60e8b8c745..1055164c6232db23e34644aaa9aa39c69753d506 100644 (file)
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1312,6 +1312,7 @@ static void tipc_link_build_proto_msg(struct tipc_link *l, int mtyp, bool probe,
        msg_set_next_sent(hdr, l->snd_nxt);
        msg_set_ack(hdr, l->rcv_nxt - 1);
        msg_set_bcast_ack(hdr, bcl->rcv_nxt - 1);
+       msg_set_bc_ack_invalid(hdr, !node_up);
        msg_set_last_bcast(hdr, l->bc_sndlink->snd_nxt - 1);
        msg_set_link_tolerance(hdr, tolerance);
        msg_set_linkprio(hdr, priority);
@@ -1574,6 +1575,7 @@ static void tipc_link_build_bc_init_msg(struct tipc_link *l,
        __skb_queue_head_init(&list);
        if (!tipc_link_build_bc_proto_msg(l->bc_rcvlink, false, 0, &list))
                return;
+       msg_set_bc_ack_invalid(buf_msg(skb_peek(&list)), true);
        tipc_link_xmit(l, &list, xmitq);
 }
 

diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index c3832cdf2278a3a49dd6624d350ba34096764897..50a739860d379ebaf775e566e67979dda6842e4d 100644 (file)
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -714,6 +714,23 @@ static inline void msg_set_peer_stopping(struct tipc_msg *m, u32 s)
        msg_set_bits(m, 5, 13, 0x1, s);
 }
 
+static inline bool msg_bc_ack_invalid(struct tipc_msg *m)
+{
+       switch (msg_user(m)) {
+       case BCAST_PROTOCOL:
+       case NAME_DISTRIBUTOR:
+       case LINK_PROTOCOL:
+               return msg_bits(m, 5, 14, 0x1);
+       default:
+               return false;
+       }
+}
+
+static inline void msg_set_bc_ack_invalid(struct tipc_msg *m, bool invalid)
+{
+       msg_set_bits(m, 5, 14, 0x1, invalid);
+}
+
 static inline char *msg_media_addr(struct tipc_msg *m)
 {
        return (char *)&m->hdr[TIPC_MEDIA_INFO_OFFSET];

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index a04fe9be1c60e2a7c1cb2f90c80731e08dcc910d..c1cfd92de17aee30a310305707a70ecb87fd2548 100644 (file)
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -156,6 +156,7 @@ static void named_distribute(struct net *net, struct sk_buff_head *list,
                                pr_warn("Bulk publication failure\n");
                                return;
                        }
+                       msg_set_bc_ack_invalid(buf_msg(skb), true);
                        item = (struct distr_item *)msg_data(buf_msg(skb));
                }
 

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 7ef14e2d2356590d72284e3ef056d63dee3d1b12..9d2f4c2b08abc56ecb627ff067ad359c54e735fd 100644 (file)
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1535,7 +1535,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
        if (unlikely(usr == LINK_PROTOCOL))
                tipc_node_bc_sync_rcv(n, hdr, bearer_id, &xmitq);
        else if (unlikely(tipc_link_acked(n->bc_entry.link) != bc_ack))
-               tipc_bcast_ack_rcv(net, n->bc_entry.link, bc_ack);
+               tipc_bcast_ack_rcv(net, n->bc_entry.link, hdr);
 
        /* Receive packet directly if conditions permit */
        tipc_node_read_lock(n);