Skip to content

Commit 1eed677

Browse files
lxindavem330
authored andcommitted
sctp: fix the transport dead race check by using atomic_add_unless on refcnt
Now when __sctp_lookup_association is running in BH, it will try to check if t->dead is set, but meanwhile other CPUs may be freeing this transport and this assoc and if it happens that __sctp_lookup_association checked t->dead a bit too early, it may think that the association is still good while it was already freed. So we fix this race by using atomic_add_unless in sctp_transport_hold. After we get one transport from hashtable, we will hold it only when this transport's refcnt is not 0, so that we can make sure t->asoc cannot be freed before we hold the asoc again. Note that sctp association is not freed using RCU so we can't use atomic_add_unless() with it as it may just be too late for that either. Fixes: 4f00878 ("sctp: apply rhashtable api to send/recv path") Reported-by: Vlad Yasevich <vyasevich@gmail.com> Signed-off-by: Xin Long <lucien.xin@gmail.com> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 2baaa2d commit 1eed677

File tree

3 files changed

+14
-9
lines changed

3 files changed

+14
-9
lines changed

include/net/sctp/structs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,7 @@ void sctp_transport_route(struct sctp_transport *, union sctp_addr *,
955955
void sctp_transport_pmtu(struct sctp_transport *, struct sock *sk);
956956
void sctp_transport_free(struct sctp_transport *);
957957
void sctp_transport_reset_timers(struct sctp_transport *);
958-
void sctp_transport_hold(struct sctp_transport *);
958+
int sctp_transport_hold(struct sctp_transport *);
959959
void sctp_transport_put(struct sctp_transport *);
960960
void sctp_transport_update_rto(struct sctp_transport *, __u32);
961961
void sctp_transport_raise_cwnd(struct sctp_transport *, __u32, __u32);

net/sctp/input.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -935,15 +935,22 @@ static struct sctp_association *__sctp_lookup_association(
935935
struct sctp_transport **pt)
936936
{
937937
struct sctp_transport *t;
938+
struct sctp_association *asoc = NULL;
938939

940+
rcu_read_lock();
939941
t = sctp_addrs_lookup_transport(net, local, peer);
940-
if (!t || t->dead)
941-
return NULL;
942+
if (!t || !sctp_transport_hold(t))
943+
goto out;
942944

943-
sctp_association_hold(t->asoc);
945+
asoc = t->asoc;
946+
sctp_association_hold(asoc);
944947
*pt = t;
945948

946-
return t->asoc;
949+
sctp_transport_put(t);
950+
951+
out:
952+
rcu_read_unlock();
953+
return asoc;
947954
}
948955

949956
/* Look up an association. protected by RCU read lock */
@@ -955,9 +962,7 @@ struct sctp_association *sctp_lookup_association(struct net *net,
955962
{
956963
struct sctp_association *asoc;
957964

958-
rcu_read_lock();
959965
asoc = __sctp_lookup_association(net, laddr, paddr, transportp);
960-
rcu_read_unlock();
961966

962967
return asoc;
963968
}

net/sctp/transport.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ void sctp_transport_route(struct sctp_transport *transport,
296296
}
297297

298298
/* Hold a reference to a transport. */
299-
void sctp_transport_hold(struct sctp_transport *transport)
299+
int sctp_transport_hold(struct sctp_transport *transport)
300300
{
301-
atomic_inc(&transport->refcnt);
301+
return atomic_add_unless(&transport->refcnt, 1, 0);
302302
}
303303

304304
/* Release a reference to a transport and clean up

0 commit comments

Comments
 (0)