Re:Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in del_chan
From: Gao Feng <hidden>
Date: 2017-08-07 01:37:35
At 2017-08-03 01:13:36, "Cong Wang" [off-list ref] wrote:
Hi, Gao On Tue, Aug 1, 2017 at 1:39 PM, Cong Wang [off-list ref] wrote:quoted
From my understanding, this RCU is supposed to protect the pppox_sock pointers in 'callid_sock' which could be NULL'ed in del_chan(). And the pppox_sock is freed when the last refcnt is gone, that is, when sock dctor is called. pptp_release() is ONLY called when the fd in user-space is gone, not necessarily the last refcnt.
Hi Cong, I am sorry to reply you so late, because I took a short trip recently, and didn't check my emails. I think the RCU should be supposed to avoid the race between del_chan and lookup_chan. The synchronize_rcu could make sure if there was one which calls lookup_chan in this period, it would be finished and the sock refcnt is increased if necessary. So I think it is ok to invoke sock_put directly without SOCK_RCU_FREE, because the lookup_chan caller has already hold the sock refcnt, Best Regards Feng
quoted hunk ↗ jump to hunk
Your commit is probably not the right fix. Can you try the following fix?diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c index 6dde9a0cfe76..e75bb95c107f 100644 --- a/drivers/net/ppp/pptp.c +++ b/drivers/net/ppp/pptp.c@@ -519,7 +519,6 @@ static int pptp_release(struct socket *sock) po = pppox_sk(sk); del_chan(po); - synchronize_rcu(); pppox_unbind_sock(sk); sk->sk_state = PPPOX_DEAD;@@ -564,6 +563,7 @@ static int pptp_create(struct net *net, structsocket *sock, int kern) sk->sk_family = PF_PPPOX; sk->sk_protocol = PX_PROTO_PPTP; sk->sk_destruct = pptp_sock_destruct; + sock_set_flag(sk, SOCK_RCU_FREE); po = pppox_sk(sk); opt = &po->proto.pptp;