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: [email protected]
> Signed-off-by: Guillaume Nault <[email protected]>
> ---
> 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
