[PATCH] LLC: Avoid potential NULL dereference in net/llc/af_llc.c::llc_ui_accept() .

[PATCH] LLC: Avoid potential NULL dereference in net/llc/af_llc.c::llc_ui_accept() .

[Jesper Juhl]

Since skb_dequeue() may return NULL we risk dereferencing a NULL pointer at
  if (!skb->sk)
This patch avoids that by also testing for a NULL skb.


Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 2652ead..a3c885f 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -620,7 +620,7 @@ static int llc_ui_accept(struct socket *
 	        llc_sk(sk)->laddr.lsap);
 	skb = skb_dequeue(&sk->sk_receive_queue);
 	rc = -EINVAL;
-	if (!skb->sk)
+	if (!skb || !skb->sk)
 		goto frees;
 	rc = 0;
 	newsk = skb->sk;

Re: [PATCH] LLC: Avoid potential NULL dereference in net/llc/af_llc.c::llc_ui_accept() .

[David Miller]

From: Jesper Juhl <jesper.juhl@gmail.com>
Date: Thu, 2 Nov 2006 01:21:53 +0100

> Since skb_dequeue() may return NULL we risk dereferencing a NULL pointer at
>   if (!skb->sk)
> This patch avoids that by also testing for a NULL skb.
> 
> 
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>

It can't return NULL in this context because we just checked
skb_queue_empty() with the socket lock held and llc_wait_data()
will return zero only if skb_queue_empty() is false.

I know it's hard for automated tools to see this, but it's not
reasonable to put this extra check in there since it is
superfluous due to the above mentioned invariants.

Re: [PATCH] LLC: Avoid potential NULL dereference in net/llc/af_llc.c::llc_ui_accept() .

[Jesper Juhl]

On 02/11/06, David Miller <davem@davemloft.net> wrote:
> From: Jesper Juhl <jesper.juhl@gmail.com>
> Date: Thu, 2 Nov 2006 01:21:53 +0100
>
> > Since skb_dequeue() may return NULL we risk dereferencing a NULL pointer at
> >   if (!skb->sk)
> > This patch avoids that by also testing for a NULL skb.
> >
> >
> > Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
>
> It can't return NULL in this context because we just checked
> skb_queue_empty() with the socket lock held and llc_wait_data()
> will return zero only if skb_queue_empty() is false.
>
> I know it's hard for automated tools to see this, but it's not
> reasonable to put this extra check in there since it is
> superfluous due to the above mentioned invariants.
>
Fair enough. Ignore the patch.
Thank you for the explanation.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html