netdev - Re: 2.6.23-rc8 network problem. Mem leak? ip1000a?

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:	Mon, 07 Jan 2008 23:14:47 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	linux@...izon.com
Cc:	akpm@...ux-foundation.org, netdev@...r.kernel.org,
	romieu@...zoreil.com
Subject: Re: 2.6.23-rc8 network problem. Mem leak? ip1000a?

From: David Miller <davem@...emloft.net>
Date: Mon, 07 Jan 2008 23:07:09 -0800 (PST)

> From: linux@...izon.com
> Date: 8 Jan 2008 01:52:11 -0500
> 
> > @@ -172,6 +172,10 @@ config IP1000
> >  	select MII
> >  	---help---
> >  	  This driver supports IP1000 gigabit Ethernet cards.
> > +	  It works, but suffers from a memory leak.  Signifcant
> > +	  use will consume unswappable kernel memory until the
> > +	  machine runs out of memory and crashes.  Thus, this
> > +	  driver cannot be considered usable at the the present time.
> 
> This is not how we handle and track bugs.
> 
> Such a patch is inappropriate, and I'd like to ask that you just be
> patient until someone has a chance to try and figure out what the
> problem is.  Or even better, you can try to track down the problem
> yourself since you seem to have a specific interest in this problem.

Actually, the bug is amazingly obvious after a quick scan of this
driver.

ipg_nic_rx_free_skb() is called from various places and is given zero
context to work with.  It assumes that the caller wants
"sp->rx_current % IPG_RFCLIST_LENGTH" to be freed.

But that's not right in most cases.  For example, consider the call in
ipg_nic_rx_with_end().  This function is invoked from ipg_nic_rx()
like so:

	unsigned int curr = sp->rx_current;
 ...
	for (i = 0; i < IPG_MAXRFDPROCESS_COUNT; i++, curr++) {
		unsigned int entry = curr % IPG_RFDLIST_LENGTH;
		struct ipg_rx *rxfd = sp->rxd + entry;

		if (!(rxfd->rfs & le64_to_cpu(IPG_RFS_RFDDONE)))
			break;

		switch (ipg_nic_rx_check_frame_type(dev)) {
 ...
		case Frame_WithEnd:
			ipg_nic_rx_with_end(dev, tp, rxfd, entry);
			break;
 ...
		}
	}

	sp->rx_current = curr;

So sp->rx_current does not correspond to the packet being processed
currently, so ipg_nic_rx_free_skb() will only look at and try to free
only the first packet the above loop tries to processe.

WOW!!!!  Amazing!!!

I invested 30 seconds of code reading to figure out the leak.  A much
better investment of time than adding bogus comments to the Kconfig
help text don't you think? :-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html