Re: [PATCH] ISDN: Avoid a potential NULL ptr deref in ippp

[David Rientjes <rientjes@cs.washington.edu>]

On Tue, 21 Nov 2006, Jesper Juhl wrote:

> Ok, I see your point. There may not be an actual bug here, but
> couldn't it still be considered an improvement, given that with my
> patch we'll  a) print a warning that we ran into a memory shortage
> problem, and  b) we save a call to ipc->decompress() and some switch
> logic in the failing case.   ???
> 

No, because there is duplication of code.  This error condition is already 
addressed:

	skb_out = dev_alloc_skb(is->mru + PPP_HDRLEN);
	len = ipc->decompress(stat, skb, skb_out, &rsparm);
	kfree_skb(skb);
	if (len <= 0) {
		switch (len) {
		case DECOMP_ERROR:
			...
			break;
		case DECOMP_FATALERROR:
			...
			break;
		}
		kfree_skb(skb_out);
		return NULL;
	}

Since neither DECOMP_ERROR or DECOMP_FATALERROR represent a NULL return 
from ipc->decompress(), the switch clause is a no-op and skb_out is freed 
and NULL is returned.

The only thing your patch addresses is moving this before the 
ipc->decompress() call and _duplicating_ both the skb free and the return 
code, as well as adding a warning.  The warning is unnecessary because OOM 
killer will be called soon anyway if this condition is ever reached so the 
fact that it was this allocation that could not be satisfied doesn't 
matter.  Likewise, we need the return code from ipc->decompress() to do 
the other error checking involved.

		David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/