public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Manfred Spraul <manfred@colorfullife.com>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: Linus Torvalds <torvalds@osdl.org>,
	Ayaz Abdulla <AAbdulla@nvidia.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Netdev <netdev@oss.sgi.com>
Subject: Re: [PATCH] forcedeth: fix random memory scribbling bug
Date: Sat, 24 Dec 2005 17:08:10 +0100	[thread overview]
Message-ID: <43AD726A.5010703@colorfullife.com> (raw)
In-Reply-To: <43AD64AB.2070306@pobox.com>

Jeff Garzik wrote:

> Manfred Spraul wrote:
>
>> Two critical bugs were found in forcedeth 0.47:
>> - TSO doesn't work.
>> - pci_map_single() for the rx buffers is called with size==0. This 
>> bug is critical, it causes random memory corruptions on systems with 
>> an iommu.
>>
>> Below is a minimal fix for both bugs, for inclusion into 2.6.15.
>> TSO will be fixed properly in the next version.
>> Tested on x86-64.
>>
>> Signed-Off-By: Manfred Spraul <manfred@colorfullife.com>
>
>
> 1) Why does forcedeth require a non-standard calculation for each 
> pci_map_single() call?
>
- skb->len is the wrong thing (tm), since it's 0 until skb_put().
- I have not found a field that contains the actual size of the data 
area of an skb.
- the results must be identical for map and unmap.
- I could recalculate the size of the allocation from np->rx_buf_sz, but 
I don't like that. Right now it would work, but it's too subtile that 
changing rx_buf_sz while there are outstanding rx buffers results in a 
iommu memory leak.
Therefore I decided to calculate the mapping size with "skb->end - 
skb->data": The size of the mapping for an skb is calculated by looking 
at fields in the skb, no knowledge about driver fields.

> 2) I have requested multiple times that you avoid MIME...
>
It's the first time that you complain about Content-Transfer-Encoding: 
7bit attachments.

> 3) Why disable TSO completely?  It sounds like it should default to 
> off, then permit enabling via ethtool.
>
The bugfix is in 0.49 - it's just a bit larger, I would consider it for 
2.5.16.

--
    Manfred

  reply	other threads:[~2005-12-24 16:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-24 13:19 [PATCH] forcedeth: fix random memory scribbling bug Manfred Spraul
2005-12-24 15:09 ` Jeff Garzik
2005-12-24 16:08   ` Manfred Spraul [this message]
2005-12-24 19:57     ` Linus Torvalds
2005-12-24 19:52 ` Linus Torvalds
2005-12-24 19:56   ` Manfred Spraul
2005-12-24 20:41     ` Linus Torvalds
2005-12-24 21:06       ` Jeff Garzik
2005-12-24 21:20         ` Francois Romieu
2005-12-24 19:58   ` Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43AD726A.5010703@colorfullife.com \
    --to=manfred@colorfullife.com \
    --cc=AAbdulla@nvidia.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@oss.sgi.com \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox