diff mbox

[net,stable,1/2] l2tp: Fix PPP header erasure and memory leak

Message ID 20130612140723.GB3500@alphalink.fr
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Guillaume Nault June 12, 2013, 2:07 p.m. UTC
Copy user data after PPP framing header. This prevents erasure of the
added PPP header and avoids leaking two bytes of uninitialised memory
at the end of skb's data buffer.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Eric Dumazet June 12, 2013, 2:41 p.m. UTC | #1
On Wed, 2013-06-12 at 16:07 +0200, Guillaume Nault wrote:
> Copy user data after PPP framing header. This prevents erasure of the
> added PPP header and avoids leaking two bytes of uninitialised memory
> at the end of skb's data buffer.
> 
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> ---
>  net/l2tp/l2tp_ppp.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> index 637a341..681c626 100644
> --- a/net/l2tp/l2tp_ppp.c
> +++ b/net/l2tp/l2tp_ppp.c
> @@ -346,12 +346,12 @@ static int pppol2tp_sendmsg(struct kiocb *iocb, struct socket *sock, struct msgh
>  	skb_put(skb, 2);
>  
>  	/* Copy user data into skb */
> -	error = memcpy_fromiovec(skb->data, m->msg_iov, total_len);
> +	error = memcpy_fromiovec(skb_put(skb, total_len), m->msg_iov,
> +				 total_len);
>  	if (error < 0) {
>  		kfree_skb(skb);
>  		goto error_put_sess_tun;
>  	}
> -	skb_put(skb, total_len);
>  
>  	l2tp_xmit_skb(session, skb, session->hdr_len);
>  

I see no real change in your patch.

skb_put(skb, X) returns skb->data before the put operation.

Could you elaborate ?



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet June 12, 2013, 2:50 p.m. UTC | #2
On Wed, 2013-06-12 at 07:41 -0700, Eric Dumazet wrote:
> On Wed, 2013-06-12 at 16:07 +0200, Guillaume Nault wrote:
> > Copy user data after PPP framing header. This prevents erasure of the
> > added PPP header and avoids leaking two bytes of uninitialised memory
> > at the end of skb's data buffer.
> > 
> > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> > ---
> >  net/l2tp/l2tp_ppp.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> > index 637a341..681c626 100644
> > --- a/net/l2tp/l2tp_ppp.c
> > +++ b/net/l2tp/l2tp_ppp.c
> > @@ -346,12 +346,12 @@ static int pppol2tp_sendmsg(struct kiocb *iocb, struct socket *sock, struct msgh
> >  	skb_put(skb, 2);
> >  
> >  	/* Copy user data into skb */
> > -	error = memcpy_fromiovec(skb->data, m->msg_iov, total_len);
> > +	error = memcpy_fromiovec(skb_put(skb, total_len), m->msg_iov,
> > +				 total_len);
> >  	if (error < 0) {
> >  		kfree_skb(skb);
> >  		goto error_put_sess_tun;
> >  	}
> > -	skb_put(skb, total_len);
> >  
> >  	l2tp_xmit_skb(session, skb, session->hdr_len);
> >  
> 
> I see no real change in your patch.
> 
> skb_put(skb, X) returns skb->data before the put operation.
> 
> Could you elaborate ?

Oh well, I missed the prior skb_put(skb, 2), nevermind


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 13, 2013, 9:39 a.m. UTC | #3
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Wed, 12 Jun 2013 16:07:23 +0200

> Copy user data after PPP framing header. This prevents erasure of the
> added PPP header and avoids leaking two bytes of uninitialised memory
> at the end of skb's data buffer.
> 
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 637a341..681c626 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -346,12 +346,12 @@  static int pppol2tp_sendmsg(struct kiocb *iocb, struct socket *sock, struct msgh
 	skb_put(skb, 2);
 
 	/* Copy user data into skb */
-	error = memcpy_fromiovec(skb->data, m->msg_iov, total_len);
+	error = memcpy_fromiovec(skb_put(skb, total_len), m->msg_iov,
+				 total_len);
 	if (error < 0) {
 		kfree_skb(skb);
 		goto error_put_sess_tun;
 	}
-	skb_put(skb, total_len);
 
 	l2tp_xmit_skb(session, skb, session->hdr_len);