[PATCH] llc_cmsg_rcv was getting called after sk_eat_skb.

[PATCH] llc_cmsg_rcv was getting called after sk_eat_skb.

[Alexandru Juncu]

Received non stream protocol packets were calling llc_cmsg_rcv that used a
skb after that skb was released by sk_eat_skb. This caused received STP
packets to generate kernel panics.

Signed-off-by: Alexandru Juncu <ajuncu@ixiacom.com>
Signed-off-by: Kunjan Naik <knaik@ixiacom.com>
---
 net/llc/af_llc.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index dfd3a64..a18e6c3 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -833,15 +833,15 @@ static int llc_ui_recvmsg(struct kiocb *iocb, struct socket *sock,
 		copied += used;
 		len -= used;
 
+		/* For non stream protcols we get one packet per recvmsg call */
+		if (sk->sk_type != SOCK_STREAM)
+			goto copy_uaddr;
+
 		if (!(flags & MSG_PEEK)) {
 			sk_eat_skb(sk, skb, 0);
 			*seq = 0;
 		}
 
-		/* For non stream protcols we get one packet per recvmsg call */
-		if (sk->sk_type != SOCK_STREAM)
-			goto copy_uaddr;
-
 		/* Partial read */
 		if (used + offset < skb->len)
 			continue;
@@ -857,6 +857,12 @@ copy_uaddr:
 	}
 	if (llc_sk(sk)->cmsg_flags)
 		llc_cmsg_rcv(msg, skb);
+
+	if (!(flags & MSG_PEEK)) {
+			sk_eat_skb(sk, skb, 0);
+			*seq = 0;
+	}
+
 	goto out;
 }
 
-- 
1.7.4.1

Re: [PATCH] llc_cmsg_rcv was getting called after sk_eat_skb.

[Daniel Baluta]

Hello Alex,

Please add a llc: prefix to the Subject. Also the title
should suggest what is the patch supposed to do.

e.g:

llc: Don't call llc_cmsg_rcv after sk_eat_skb

On Fri, Dec 16, 2011 at 11:01 AM, Alexandru Juncu <ajuncu@ixiacom.com> wrote:
> Received non stream protocol packets were calling llc_cmsg_rcv that used a
> skb after that skb was released by sk_eat_skb. This caused received STP
> packets to generate kernel panics.
>
> Signed-off-by: Alexandru Juncu <ajuncu@ixiacom.com>
> Signed-off-by: Kunjan Naik <knaik@ixiacom.com>
> ---
>  net/llc/af_llc.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
> index dfd3a64..a18e6c3 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c
> @@ -833,15 +833,15 @@ static int llc_ui_recvmsg(struct kiocb *iocb, struct socket *sock,
>                copied += used;
>                len -= used;
>
> +               /* For non stream protcols we get one packet per recvmsg call */
> +               if (sk->sk_type != SOCK_STREAM)
> +                       goto copy_uaddr;
> +
>                if (!(flags & MSG_PEEK)) {
>                        sk_eat_skb(sk, skb, 0);
>                        *seq = 0;
>                }
>
> -               /* For non stream protcols we get one packet per recvmsg call */
> -               if (sk->sk_type != SOCK_STREAM)
> -                       goto copy_uaddr;
> -
>                /* Partial read */
>                if (used + offset < skb->len)
>                        continue;
> @@ -857,6 +857,12 @@ copy_uaddr:
>        }
>        if (llc_sk(sk)->cmsg_flags)
>                llc_cmsg_rcv(msg, skb);
> +
> +       if (!(flags & MSG_PEEK)) {
> +                       sk_eat_skb(sk, skb, 0);
> +                       *seq = 0;
> +       }
> +
>        goto out;
>  }
>
> --
> 1.7.4.1

David, Eric could you please review this?

thanks,
Daniel.

Re: [PATCH] llc_cmsg_rcv was getting called after sk_eat_skb.

[David Miller]

From: Alexandru Juncu <ajuncu@ixiacom.com>
Date: Fri, 16 Dec 2011 11:01:25 +0200

> Received non stream protocol packets were calling llc_cmsg_rcv that used a
> skb after that skb was released by sk_eat_skb. This caused received STP
> packets to generate kernel panics.
> 
> Signed-off-by: Alexandru Juncu <ajuncu@ixiacom.com>
> Signed-off-by: Kunjan Naik <knaik@ixiacom.com>

This bug was introduced by commit 30a584d944fbd599d4a8f470f75bf7af1a15b466:

commit 30a584d944fbd599d4a8f470f75bf7af1a15b466
Author: Stephen Hemminger <shemminger@osdl.org>
Date:   Thu Aug 3 16:38:49 2006 -0700

    [LLX]: SOCK_DGRAM interface fixes

The new logic added by this fix is slightly clumsy, but seems correct
and I can't suggest a better fix at this time.

I'll apply this, thanks.