| Message ID | CAFcO6XMgwuz97EJN+8jh9PJ9seaUbousDBOh9sduM6MZ6MRHxA@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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.
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
From 5a2d0282931967dc9d90248221b3120e1e33551c Mon Sep 17 00:00:00 2001 From: Xiaolong Huang <butterflyhuangxx@gmail.com> Date: Wed, 3 Nov 2021 23:33:55 +0800 Subject: [PATCH] net: ppp: pppoe: fix a kernel-infoleak in pppoe_getname() The struct sockaddr_pppox has a 2-byte hole, and pppoe_getname() currently does not clear it before copying kernel data to user space. Signed-off-by: Xiaolong Huang <butterflyhuangxx@gmail.com> --- drivers/net/ppp/pppoe.c | 5 +++++ 1 file changed, 5 insertions(+) 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, -- 2.25.1
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 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. ``` memcpy(&sp.sa_addr.pppoe, &pppox_sk(sock->sk)->pppoe_pa, ``` The attachment is a patch. Regards, butt3rflyh4ck.