Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAFcO6XMgwuz97EJN+8jh9PJ9seaUbousDBOh9sduM6MZ6MRHxA@mail.gmail.com>
 <20211103095308.7ff68a7f@kicinski-fedora-PC1C0HJN>
In-Reply-To: <20211103095308.7ff68a7f@kicinski-fedora-PC1C0HJN>
From:   butt3rflyh4ck <butterflyhuangxx@gmail.com>
Date:   Thu, 4 Nov 2021 11:07:16 +0800
Message-ID: <CAFcO6XPA5uR6PLqRYxNmie6jZkkZxpkyTzJmB7pyNNf17gg3Wg@mail.gmail.com>
Subject: Re: A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     mostrows@earthlink.net, "David S. Miller" <davem@davemloft.net>,
        Networking <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Bhaskar Chowdhury <unixbhaskar@gmail.com>,
        Guillaume Nault <gnault@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Ok, I will check. Oh, you are right.


On Thu, Nov 4, 2021 at 12:53 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 4 Nov 2021 00:14:31 +0800 butt3rflyh4ck wrote:
> > Hi, I report a kernel-infoleak bug in pppoe_getname()) in
> > drivers/net/ppp/pppoe.c.
> > And we can call getname ioctl to invoke pppoe_getname().
> >
> > ###anaylze
> > ```
> > static int pppoe_getname(struct socket *sock, struct sockaddr *uaddr,
> >   int peer)
> > {
> > int len = sizeof(struct sockaddr_pppox);
> > struct sockaddr_pppox sp;    ///--->  define a 'sp' in stack but does
> > not clear it
> >
> > sp.sa_family = AF_PPPOX;   ///---> sp.sa_family is a short type, just
>
> But the structure is marked as __packed.
>
> > 2 byte sizes.
> > sp.sa_protocol = PX_PROTO_OE;
> > memcpy(&sp.sa_addr.pppoe, &pppox_sk(sock->sk)->pppoe_pa,
> >        sizeof(struct pppoe_addr));
> >
> > memcpy(uaddr, &sp, len);
> >
> > return len;
> > }
> > ```
> > There is an anonymous 2-byte hole after sa_family, make sure to clear it.
> >
> > ###fix
> > use memset() to clear the struct sockaddr_pppox sp.
> > ```
> > diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> > index 3619520340b7..fec328ad7202 100644
> > --- a/drivers/net/ppp/pppoe.c
> > +++ b/drivers/net/ppp/pppoe.c
> > @@ -723,6 +723,11 @@ static int pppoe_getname(struct socket *sock,
> > struct sockaddr *uaddr,
> >         int len = sizeof(struct sockaddr_pppox);
> >         struct sockaddr_pppox sp;
> >
> > +       /* There is an anonymous 2-byte hole after sa_family,
> > +        * make sure to clear it.
> > +        */
> > +       memset(&sp, 0, len);
> > +
> >         sp.sa_family    = AF_PPPOX;
> >         sp.sa_protocol  = PX_PROTO_OE;
> >         memcpy(&sp.sa_addr.pppoe, &pppox_sk(sock->sk)->pppoe_pa,
> > ```
> > The attachment is a patch.
>


--
Active Defense Lab of Venustech