From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Easton Subject: Re: [PATCH net] pppoe: check sockaddr length in pppoe_connect() Date: Fri, 27 Apr 2018 08:23:16 -0400 Message-ID: <20180427122316.GA20688@la.guarana.org> References: <387ca48810af36f2626049008a795d1adc375cb8.1524494257.git.g.nault@alphalink.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, Michal Ostrowski To: Guillaume Nault Return-path: Received: from la.guarana.org ([173.254.219.205]:60250 "EHLO la.guarana.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752808AbeD0MXQ (ORCPT ); Fri, 27 Apr 2018 08:23:16 -0400 Content-Disposition: inline In-Reply-To: <387ca48810af36f2626049008a795d1adc375cb8.1524494257.git.g.nault@alphalink.fr> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, Apr 23, 2018 at 04:38:27PM +0200, Guillaume Nault wrote: > We must validate sockaddr_len, otherwise userspace can pass fewer data > than we expect and we end up accessing invalid data. > > Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers") > Reported-by: syzbot+4f03bdf92fdf9ef5ddab@syzkaller.appspotmail.com > Signed-off-by: Guillaume Nault > --- > drivers/net/ppp/pppoe.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c > index 1483bc7b01e1..7df07337d69c 100644 > --- a/drivers/net/ppp/pppoe.c > +++ b/drivers/net/ppp/pppoe.c > @@ -620,6 +620,10 @@ static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr, > lock_sock(sk); > > error = -EINVAL; > + > + if (sockaddr_len != sizeof(struct sockaddr_pppox)) > + goto end; > + > if (sp->sa_protocol != PX_PROTO_OE) > goto end; There's another bug here - pppoe_connect() should also be validating sp->sa_family. My suggested patch was going to be: diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c index 1483bc7..90eb3fd 100644 --- a/drivers/net/ppp/pppoe.c +++ b/drivers/net/ppp/pppoe.c @@ -620,6 +620,14 @@ static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr, lock_sock(sk); error = -EINVAL; + if (sockaddr_len < sizeof(struct sockaddr_pppox)) + goto end; + + error = -EAFNOSUPPORT; + if (sp->sa_family != AF_PPPOX) + goto end; + + error = -EINVAL; if (sp->sa_protocol != PX_PROTO_OE) goto end; Should I rework this on top of net.git HEAD? (The same applies to pppol2tp_connect()). - Kevin