nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

sctp: fix the issue sctp_diag uses lock_sock in rcu_read_lock

Browse source 
When sctp dumps all the ep->assocs, it needs to lock_sock first,
but now it locks sock in rcu_read_lock, and lock_sock may sleep,
which would break rcu_read_lock.

This patch is to get and hold one sock when traversing the list.
After that and get out of rcu_read_lock, lock and dump it. Then
it will traverse the list again to get the next one until all
sctp socks are dumped.

For sctp_diag_dump_one, it fixes this issue by holding asoc and
moving cb() out of rcu_read_lock in sctp_transport_lookup_process.

Fixes: 8f840e47f1 ("sctp: add the sctp_diag.c file")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Xin Long 2016-09-29 02:55:44 +08:00 committed by David S. Miller
parent 75b005b949
commit 1cceda7849
2 changed files with 47 additions and 21 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

58
net/sctp/sctp_diag.c
View file

@ -272,28 +272,17 @@ static int sctp_tsp_dump_one(struct sctp_transport *tsp, void *p)
return err; return err;
} }
static int sctp_tsp_dump(struct sctp_transport *tsp, void *p) static int sctp_sock_dump(struct sock *sk, void *p)
{ {
struct sctp_endpoint *ep = tsp->asoc->ep; struct sctp_endpoint *ep = sctp_sk(sk)->ep;
struct sctp_comm_param *commp = p; struct sctp_comm_param *commp = p;
struct sock *sk = ep->base.sk;
struct sk_buff *skb = commp->skb; struct sk_buff *skb = commp->skb;
struct netlink_callback *cb = commp->cb; struct netlink_callback *cb = commp->cb;
const struct inet_diag_req_v2 *r = commp->r; const struct inet_diag_req_v2 *r = commp->r;
struct sctp_association *assoc = struct sctp_association *assoc;
list_entry(ep->asocs.next, struct sctp_association, asocs);
int err = 0; int err = 0;
/* find the ep only once through the transports by this condition */
if (tsp->asoc != assoc)
goto out;
if (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)
goto out;
lock_sock(sk); lock_sock(sk);
if (sk != assoc->base.sk)
goto release;
list_for_each_entry(assoc, &ep->asocs, asocs) { list_for_each_entry(assoc, &ep->asocs, asocs) {
if (cb->args[4] < cb->args[1]) if (cb->args[4] < cb->args[1])
goto next; goto next;
@ -312,7 +301,7 @@ static int sctp_tsp_dump(struct sctp_transport *tsp, void *p)
cb->nlh->nlmsg_seq, cb->nlh->nlmsg_seq,
NLM_F_MULTI, cb->nlh) < 0) { NLM_F_MULTI, cb->nlh) < 0) {
cb->args[3] = 1; cb->args[3] = 1;
err = 2; err = 1;
goto release; goto release;
} }
cb->args[3] = 1; cb->args[3] = 1;
@ -321,7 +310,7 @@ static int sctp_tsp_dump(struct sctp_transport *tsp, void *p)
sk_user_ns(NETLINK_CB(cb->skb).sk), sk_user_ns(NETLINK_CB(cb->skb).sk),
NETLINK_CB(cb->skb).portid, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, 0, cb->nlh) < 0) { cb->nlh->nlmsg_seq, 0, cb->nlh) < 0) {
err = 2; err = 1;
goto release; goto release;
} }
next: next:
@ -333,10 +322,35 @@ static int sctp_tsp_dump(struct sctp_transport *tsp, void *p)
cb->args[4] = 0; cb->args[4] = 0;
release: release:
release_sock(sk); release_sock(sk);
sock_put(sk);
return err; return err;
}
static int sctp_get_sock(struct sctp_transport *tsp, void *p)
{
struct sctp_endpoint *ep = tsp->asoc->ep;
struct sctp_comm_param *commp = p;
struct sock *sk = ep->base.sk;
struct netlink_callback *cb = commp->cb;
const struct inet_diag_req_v2 *r = commp->r;
struct sctp_association *assoc =
list_entry(ep->asocs.next, struct sctp_association, asocs);
/* find the ep only once through the transports by this condition */
if (tsp->asoc != assoc)
goto out;
if (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)
goto out;
sock_hold(sk);
cb->args[5] = (long)sk;
return 1;
out: out:
cb->args[2]++; cb->args[2]++;
return err; return 0;
} }
static int sctp_ep_dump(struct sctp_endpoint *ep, void *p) static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
@ -472,10 +486,18 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
* 2 : to record the transport pos of this time's traversal * 2 : to record the transport pos of this time's traversal
* 3 : to mark if we have dumped the ep info of the current asoc * 3 : to mark if we have dumped the ep info of the current asoc
* 4 : to work as a temporary variable to traversal list * 4 : to work as a temporary variable to traversal list
* 5 : to save the sk we get from travelsing the tsp list.
*/ */
if (!(idiag_states & ~(TCPF_LISTEN | TCPF_CLOSE))) if (!(idiag_states & ~(TCPF_LISTEN | TCPF_CLOSE)))
goto done; goto done;
sctp_for_each_transport(sctp_tsp_dump, net, cb->args[2], &commp);
next:
cb->args[5] = 0;
sctp_for_each_transport(sctp_get_sock, net, cb->args[2], &commp);
if (cb->args[5] && !sctp_sock_dump((struct sock *)cb->args[5], &commp))
goto next;
done: done:
cb->args[1] = cb->args[4]; cb->args[1] = cb->args[4];
cb->args[4] = 0; cb->args[4] = 0;

10
net/sctp/socket.c
View file

@ -4469,17 +4469,21 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
const union sctp_addr *paddr, void *p) const union sctp_addr *paddr, void *p)
{ {
struct sctp_transport *transport; struct sctp_transport *transport;
int err = 0; int err = -ENOENT;
rcu_read_lock(); rcu_read_lock();
transport = sctp_addrs_lookup_transport(net, laddr, paddr); transport = sctp_addrs_lookup_transport(net, laddr, paddr);
if (!transport || !sctp_transport_hold(transport)) if (!transport || !sctp_transport_hold(transport))
goto out; goto out;
err = cb(transport, p);
sctp_association_hold(transport->asoc);
sctp_transport_put(transport); sctp_transport_put(transport);
out:
rcu_read_unlock(); rcu_read_unlock();
err = cb(transport, p);
sctp_association_put(transport->asoc);
out:
return err; return err;
} }
EXPORT_SYMBOL_GPL(sctp_transport_lookup_process); EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);