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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Nov 2006 14:49:41 -0800 (PST)
From:	David Rientjes <rientjes@...washington.edu>
To:	Jesper Juhl <jesper.juhl@...il.com>
cc:	linux-kernel@...r.kernel.org,
	Michael Hipp <Michael.Hipp@...dent.uni-tuebingen.de>,
	Karsten Keil <kkeil@...e.de>,
	Kai Germaschewski <kai.germaschewski@....de>,
	isdn4linux@...tserv.isdn4linux.de, starvik@...s.com,
	dev-etrax@...s.com
Subject: Re: [PATCH] ISDN: Avoid a potential NULL ptr deref in ippp

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/