netdev - Re:Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in del

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <31c9755c.ff6.15dba5238fc.Coremail.gfree.wind@vip.163.com>
Date:   Mon, 7 Aug 2017 09:32:17 +0800 (CST)
From:   "Gao Feng" <gfree.wind@....163.com>
To:     "Cong Wang" <xiyou.wangcong@...il.com>
Cc:     xeb@...l.ru, "David Miller" <davem@...emloft.net>,
        "Linux Kernel Network Developers" <netdev@...r.kernel.org>
Subject: Re:Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in
 del_chan

At 2017-08-03 01:13:36, "Cong Wang" <xiyou.wangcong@...il.com> wrote:
>Hi, Gao
>
>On Tue, Aug 1, 2017 at 1:39 PM, Cong Wang <xiyou.wangcong@...il.com> wrote:
>> 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

>
>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, struct
>socket *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;