Re: [2.6 patch] net/llc/llc_conn.c: fix possible NULL dereference
From: Eugene Teo [EMAIL PROTECTED] Date: Sat, 19 May 2007 13:49:11 +0800 Spotted by the Coverity checker. Why am I not surprised :-( There is no bug here, if Coverity warns every single time skb_peek() is used and not tested against NULL, that's a very serious shortcoming of Coverity or what it has been taught about SKB queues. It needs to learn that skb_queue_len() != 0 implies skb_peek() will not return NULL sans locking errors. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] net/llc/llc_conn.c: fix possible NULL dereference
Eugene Teo [EMAIL PROTECTED] wrote: diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c index 3b8cfbe..28a3994 100644 --- a/net/llc/llc_conn.c +++ b/net/llc/llc_conn.c @@ -323,7 +323,8 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked) if (!q_len) goto out; - skb = skb_peek(llc-pdu_unack_q); + if (! (skb = skb_peek(llc-pdu_unack_q))) + goto out; Actually we just checked that the queue length is non-zero so there must be a packet there unless someone's just removed it. If it were possible for someone else to remove it in parallel, then we've got bigger problems to worry about :) Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED] Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] net/llc/llc_conn.c: fix possible NULL dereference
Randy Dunlap wrote: On Sat, 19 May 2007 13:13:07 +0800 Eugene Teo wrote: skb_peek() might return an empty list. skb should be checked before calling llc_pdu_sn_hdr() with it. Spotted by the Coverity checker. Signed-off-by: Eugene Teo [EMAIL PROTECTED] [...] Oh, and your patch has spaces instead of tabs. It's a hassle to get thunderbird to send a patch that preserves tabs. See if this: http://mbligh.org/linuxdocs/Email/Clients/Thunderbird helps you any. Here's a resend: skb_peek() might return an empty list. skb should be checked before calling llc_pdu_sn_hdr() with it. Spotted by the Coverity checker. Signed-off-by: Eugene Teo [EMAIL PROTECTED] diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c index 3b8cfbe..6d3a07e 100644 --- a/net/llc/llc_conn.c +++ b/net/llc/llc_conn.c @@ -324,6 +324,8 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked) if (!q_len) goto out; skb = skb_peek(llc-pdu_unack_q); + if (!skb) + goto out; pdu = llc_pdu_sn_hdr(skb); /* finding position of last acked pdu in queue */