* A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c
@ 2021-11-03 16:14 butt3rflyh4ck
2021-11-03 16:53 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: butt3rflyh4ck @ 2021-11-03 16:14 UTC (permalink / raw)
To: mostrows, David S. Miller, Jakub Kicinski; +Cc: Networking, LKML
[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]
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.
```
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.
Regards,
butt3rflyh4ck.
--
Active Defense Lab of Venustech
[-- Attachment #2: 0001-net-ppp-pppoe-fix-a-kernel-infoleak-in-pppoe_getname.patch --]
[-- Type: text/x-patch, Size: 1074 bytes --]
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c
2021-11-03 16:14 A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c butt3rflyh4ck
@ 2021-11-03 16:53 ` Jakub Kicinski
2021-11-04 3:07 ` butt3rflyh4ck
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2021-11-03 16:53 UTC (permalink / raw)
To: butt3rflyh4ck
Cc: mostrows, David S. Miller, Networking, LKML, Bhaskar Chowdhury,
Guillaume Nault
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c
2021-11-03 16:53 ` Jakub Kicinski
@ 2021-11-04 3:07 ` butt3rflyh4ck
0 siblings, 0 replies; 3+ messages in thread
From: butt3rflyh4ck @ 2021-11-04 3:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: mostrows, David S. Miller, Networking, LKML, Bhaskar Chowdhury,
Guillaume Nault
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-04 3:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 16:14 A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c butt3rflyh4ck
2021-11-03 16:53 ` Jakub Kicinski
2021-11-04 3:07 ` butt3rflyh4ck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).