ppp: potential NULL dereference in ppp_mp_explode()
PATCH 1/1 ppp: potential NULL dereference in ppp_mp_explode()
Smatch complains because we check whether "pch->chan" is NULL and then dereference it unconditionally on the next line. Partly the reason this bug was introduced is because code was too complicated. I've simplified it a little.
---
Compile tested only. Perhaps it would be better to set pch->speed to zero? The comments say that zero implies the speed hasn't been set.
drivers/net/ppp_generic.c +7 -2
@@ -1314,8 +1314,13 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb) hdrlen = (ppp->flags &SC_MP_XSHORTSEQ )?MPHDRLEN_SSN :MPHDRLEN ; i =0 ;list_for_each_entry (pch, &ppp->channels, clist) { - navail += pch->avail = (pch->chan !=NULL ); - pch->speed = pch->chan->speed; +if (pch->chan) { + pch->avail =1 ; + navail++; + pch->speed = pch->chan->speed; + }else { + pch->avail =0 ; + }if (pch->avail) {if (skb_queue_empty (&pch->file.xq) || !pch->had_frag) {
From: Dan Carpenter <[email protected]>
Date: Fri, 10 Sep 2010 13:58:10 +0200
Quoted 9 lines
Smatch complains because we check whether "
pch->chan" is NULL and then dereference it unconditionally on the next line. Partly the reason this bug was introduced is because code was too complicated. I've simplified it a little.Signed-off-by Dan Carpenter <[email protected]>---
Compile tested only. Perhaps it would be better to setpch->speedto zero? The comments say that zero implies the speed hasn't been set.
I think how you've fixed it is OK, actually this whole MP rewrite could use some more auditing :-)
Applied, thanks Dan.