From patchwork Fri Jan 22 00:39:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hannes Frederic Sowa X-Patchwork-Id: 571404 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 37C42140326 for ; Fri, 22 Jan 2016 11:40:36 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=stressinduktion.org header.i=@stressinduktion.org header.b=Pou+ucs5; dkim=pass (1024-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b=rr4synRM; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750925AbcAVAjx (ORCPT ); Thu, 21 Jan 2016 19:39:53 -0500 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:45732 "EHLO out3-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750820AbcAVAjv (ORCPT ); Thu, 21 Jan 2016 19:39:51 -0500 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 0505220640 for ; Thu, 21 Jan 2016 19:39:50 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Thu, 21 Jan 2016 19:39:51 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= stressinduktion.org; h=cc:date:from:message-id:subject:to :x-sasl-enc:x-sasl-enc; s=mesmtp; bh=wrV3wlaPQ7mSNEnnNoP/qjZPysw =; b=Pou+ucs5rzNPBeUCieuLa13tRTNKS1q0XDSBgsWnLmKzHdvx8uJ6HT5xDD2 evDySsPmD/hCtEybhTa1KPexTeKYuH9MRDECAfe4Y1ayz9qOliPpJaTLqXKlPNCw B18E+MtII/9gDhd76LwyWJhBX/naX+7zTNRRkAEN8vt0KMvo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:message-id:subject:to :x-sasl-enc:x-sasl-enc; s=smtpout; bh=wrV3wlaPQ7mSNEnnNoP/qjZPys w=; b=rr4synRM5MN34LNCnPWwjhgCWjkj6dY/3+QgqKDZMl8HRP3X6QhmJgPCR+ GcC1zPLHNndtqoROnzph3Vsgd24bqZspWYpZLOYe5EuwMwmBqpLvPexDPhKQI2dW ztuWTYWPfc+UnzgxXXQ80Ood0BlUQarhd3hYas/G8iPZcxs7Q= X-Sasl-enc: rQ9fKR6RYam5sZFtj5pN9S+py58ajsjPzeSlMkjocU8o 1453423190 Received: from z.localhost.localdomain (unknown [213.55.184.153]) by mail.messagingengine.com (Postfix) with ESMTPA id 2CD73C01717; Thu, 21 Jan 2016 19:39:49 -0500 (EST) From: Hannes Frederic Sowa To: netdev@vger.kernel.org Cc: Dmitry Kozlov , Sasha Levin , Dmitry Vyukov , Dave Jones Subject: [PATCH net] pptp: fix illegal memory access caused by multiple bind()s Date: Fri, 22 Jan 2016 01:39:43 +0100 Message-Id: <1453423183-4211-1-git-send-email-hannes@stressinduktion.org> X-Mailer: git-send-email 2.5.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Several times already this has been reported as kasan reports caused by syzkaller and trinity and people always looked at RCU races, but it is much more simple. :) In case we bind a pptp socket multiple times, we simply add it to the callid_sock list but don't remove the old binding. Thus the old socket stays in the bucket with unused call_id indexes and doesn't get cleaned up. This causes various forms of kasan reports which were hard to pinpoint. Simply don't allow multiple binds and correct error handling in pptp_bind. Also keep sk_state bits in place in pptp_connect. Fixes: 00959ade36acad ("PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)") Cc: Dmitry Kozlov Cc: Sasha Levin Cc: Dmitry Vyukov Reported-by: Dmitry Vyukov Cc: Dave Jones Reported-by: Dave Jones Signed-off-by: Hannes Frederic Sowa --- drivers/net/ppp/pptp.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c index 90868ca5e3412f..ae0905ed4a32b5 100644 --- a/drivers/net/ppp/pptp.c +++ b/drivers/net/ppp/pptp.c @@ -129,24 +129,27 @@ static int lookup_chan_dst(u16 call_id, __be32 d_addr) return i < MAX_CALLID; } -static int add_chan(struct pppox_sock *sock) +static int add_chan(struct pppox_sock *sock, + struct pptp_addr *sa) { static int call_id; spin_lock(&chan_lock); - if (!sock->proto.pptp.src_addr.call_id) { + if (!sa->call_id) { call_id = find_next_zero_bit(callid_bitmap, MAX_CALLID, call_id + 1); if (call_id == MAX_CALLID) { call_id = find_next_zero_bit(callid_bitmap, MAX_CALLID, 1); if (call_id == MAX_CALLID) goto out_err; } - sock->proto.pptp.src_addr.call_id = call_id; - } else if (test_bit(sock->proto.pptp.src_addr.call_id, callid_bitmap)) + sa->call_id = call_id; + } else if (test_bit(sa->call_id, callid_bitmap)) { goto out_err; + } - set_bit(sock->proto.pptp.src_addr.call_id, callid_bitmap); - rcu_assign_pointer(callid_sock[sock->proto.pptp.src_addr.call_id], sock); + sock->proto.pptp.src_addr = *sa; + set_bit(sa->call_id, callid_bitmap); + rcu_assign_pointer(callid_sock[sa->call_id], sock); spin_unlock(&chan_lock); return 0; @@ -416,7 +419,6 @@ static int pptp_bind(struct socket *sock, struct sockaddr *uservaddr, struct sock *sk = sock->sk; struct sockaddr_pppox *sp = (struct sockaddr_pppox *) uservaddr; struct pppox_sock *po = pppox_sk(sk); - struct pptp_opt *opt = &po->proto.pptp; int error = 0; if (sockaddr_len < sizeof(struct sockaddr_pppox)) @@ -424,10 +426,22 @@ static int pptp_bind(struct socket *sock, struct sockaddr *uservaddr, lock_sock(sk); - opt->src_addr = sp->sa_addr.pptp; - if (add_chan(po)) + if (sk->sk_state & PPPOX_DEAD) { + error = -EALREADY; + goto out; + } + + if (sk->sk_state & PPPOX_BOUND) { error = -EBUSY; + goto out; + } + + if (add_chan(po, &sp->sa_addr.pptp)) + error = -EBUSY; + else + sk->sk_state |= PPPOX_BOUND; +out: release_sock(sk); return error; } @@ -498,7 +512,7 @@ static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr, } opt->dst_addr = sp->sa_addr.pptp; - sk->sk_state = PPPOX_CONNECTED; + sk->sk_state |= PPPOX_CONNECTED; end: release_sock(sk);