netdev - Re: [PATCH net] ppp: don't override sk->sk_state in pppoe_flush

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]

Date:	Tue, 13 Oct 2015 05:13:54 +0300
From:	Denys Fedoryshchenko <nuclearcat@...learcat.com>
To:	Guillaume Nault <g.nault@...halink.fr>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Paul Mackerras <paulus@...ba.org>,
	Oleksii Berezhniak <core@....lg.ua>,
	Matt Bennett <Matt.Bennett@...iedtelesis.co.nz>
Subject: Re: [PATCH net] ppp: don't override sk->sk_state in pppoe_flush_dev()

On 2015-10-07 15:12, Guillaume Nault wrote:
> On Mon, Oct 05, 2015 at 02:08:44PM +0200, Guillaume Nault wrote:
>>  	if (po) {
>>  		struct sock *sk = sk_pppox(po);
>> 
>> -		bh_lock_sock(sk);
>> -
>> -		/* If the user has locked the socket, just ignore
>> -		 * the packet.  With the way two rcv protocols hook into
>> -		 * one socket family type, we cannot (easily) distinguish
>> -		 * what kind of SKB it is during backlog rcv.
>> -		 */
>> -		if (sock_owned_by_user(sk) == 0) {
>> -			/* We're no longer connect at the PPPOE layer,
>> -			 * and must wait for ppp channel to disconnect us.
>> -			 */
>> -			sk->sk_state = PPPOX_ZOMBIE;
>> -		}
>> -
>> -		bh_unlock_sock(sk);
>>  		if (!schedule_work(&po->proto.pppoe.padt_work))
>>  			sock_put(sk);
>>  	}
>> 
> Finally, I think I'll keep this approach for net-next, to completely
> remove PPPOX_ZOMBIE.
> For now, let's just avoid any assumption about the relationship between
> the PPPOX_ZOMBIE state and the value of po->pppoe_dev, as suggested by
> Matt.
> 
> Denys, can you let me know if your issue goes away with the following
> patch?
> ---
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 2ed7506..5e0b432 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -589,7 +589,7 @@ static int pppoe_release(struct socket *sock)
> 
>  	po = pppox_sk(sk);
> 
> -	if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND | PPPOX_ZOMBIE)) {
> +	if (po->pppoe_dev) {
>  		dev_put(po->pppoe_dev);
>  		po->pppoe_dev = NULL;
>  	}
I just got OK to upgrade server yesterday, for now around 12 hours 
working fine. I need 1-2 more days, and maybe will upgrade few more 
servers to say for sure, if it is ok or not.
Sorry for delay, just it is production servers and at current situation 
they cannot tolerate significant downtime.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html