• N
    bonding: fix igmp_retrans type and two related races · 4f5474e7
    Nikolay Aleksandrov 提交于
    First the type of igmp_retrans (which is the actual counter of
    igmp_resend parameter) is changed to u8 to be able to store values up
    to 255 (as per documentation). There are two races that were hidden
    there and which are easy to trigger after the previous fix, the first is
    between bond_resend_igmp_join_requests and bond_change_active_slave
    where igmp_retrans is set and can be altered by the periodic. The second
    race condition is between multiple running instances of the periodic
    (upon execution it can be scheduled again for immediate execution which
    can cause the counter to go < 0 which in the unsigned case leads to
    unnecessary igmp retransmissions).
    Since in bond_change_active_slave bond->lock is held for reading and
    curr_slave_lock for writing, we use curr_slave_lock for mutual
    exclusion. We can't drop them as there're cases where RTNL is not held
    when bond_change_active_slave is called. RCU is unlocked in
    bond_resend_igmp_join_requests before getting curr_slave_lock since we
    don't need it there and it's pointless to delay.
    The decrement is moved inside the "if" block because if we decrement
    unconditionally there's still a possibility for a race condition although
    it is much more difficult to hit (many changes have to happen in
    a very short period in order to trigger) which in the case of 3 parallel
    running instances of this function and igmp_retrans == 1
    (with check bond->igmp_retrans-- > 1) is:
    f1 passes, doesn't re-schedule, but decrements - igmp_retrans = 0
    f2 then passes, doesn't re-schedule, but decrements - igmp_retrans = 255
    f3 does the unnecessary retransmissions.
    Signed-off-by: NNikolay Aleksandrov <nikolay@redhat.com>
    Signed-off-by: NJay Vosburgh <fubar@us.ibm.com>
    Signed-off-by: NDavid S. Miller <davem@davemloft.net>
    4f5474e7
bond_main.c 132.8 KB