Patchwork Lucid SRU, LP562742, fix r8169 ethernet MAC address borkage

login
register
mail settings
Submitter Tim Gardner
Date May 14, 2010, 9:35 a.m.
Message ID <20100514093546.B6285F89C8@sepang.rtg.net>
Download mbox | patch
Permalink /patch/52598/
State Awaiting Upstream
Delegated to: Stefan Bader
Headers show

Comments

Tim Gardner - May 14, 2010, 9:35 a.m.
The following series of changes are clean cherry picks from upstream sufficient
and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742

The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
  Chase Douglas (1):
        UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y

are available in the git repository at:

  git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742

David Dillow (1):
      r8169: use correct barrier between cacheable and non-cacheable memory

Francois Romieu (1):
      r8169: fix broken register writes

H Hartley Sweeten (1):
      drivers/net/r8169.c: use %pM to shown MAC address

Neil Horman (2):
      r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
      r8169: clean up my printk uglyness

françois romieu (1):
      r8169: more broken register writes workaround

 drivers/net/r8169.c |   51 +++++++++++++++++++++++++++++++++++----------------
 1 files changed, 35 insertions(+), 16 deletions(-)
From a6c647a849c6dc21fb6c1a1bd83e791ad77d87a1 Mon Sep 17 00:00:00 2001
From: H Hartley Sweeten <hsweeten@visionengravers.com>
Date: Tue, 29 Dec 2009 20:10:01 -0800
Subject: [PATCH 1/6] drivers/net/r8169.c: use %pM to shown MAC address

Use the %pM kernel extension to display the MAC address.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 30a6ae8d477dc90254eb785d8ccff6dfe7d9082e)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)
Leann Ogasawara - May 19, 2010, 6:23 p.m.
On Fri, 2010-05-14 at 03:35 -0600, Tim Gardner wrote:
> The following series of changes are clean cherry picks from upstream sufficient
> and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742
> 
> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>   Chase Douglas (1):
>         UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742
> 
> David Dillow (1):
>       r8169: use correct barrier between cacheable and non-cacheable memory
> 
> Francois Romieu (1):
>       r8169: fix broken register writes
> 
> H Hartley Sweeten (1):
>       drivers/net/r8169.c: use %pM to shown MAC address
> 
> Neil Horman (2):
>       r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
>       r8169: clean up my printk uglyness
> 
> françois romieu (1):
>       r8169: more broken register writes workaround

Just want to note that all of the above patches are already included in
the actively developed Maverick kernel.  Testing feedback from the
original bug reporter for Lucid looks good.  Changes look to only touch
r8169 so widespread risk of regression seems low.  Individual
comments/ack's regarding Lucid SRU are inlined below.

>  drivers/net/r8169.c |   51 +++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 35 insertions(+), 16 deletions(-)
> From a6c647a849c6dc21fb6c1a1bd83e791ad77d87a1 Mon Sep 17 00:00:00 2001
> From: H Hartley Sweeten <hsweeten@visionengravers.com>
> Date: Tue, 29 Dec 2009 20:10:01 -0800
> Subject: [PATCH 1/6] drivers/net/r8169.c: use %pM to shown MAC address
> 
> Use the %pM kernel extension to display the MAC address.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 30a6ae8d477dc90254eb785d8ccff6dfe7d9082e)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

Small fix up to printk.  Looks sane.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |    9 ++-------
>  1 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 7b98624..2b1d500 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -3195,15 +3195,10 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (netif_msg_probe(tp)) {
>  		u32 xid = RTL_R32(TxConfig) & 0x9cf0f8ff;
>  
> -		printk(KERN_INFO "%s: %s at 0x%lx, "
> -		       "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x, "
> -		       "XID %08x IRQ %d\n",
> +		printk(KERN_INFO "%s: %s at 0x%lx, %pM, XID %08x IRQ %d\n",
>  		       dev->name,
>  		       rtl_chip_info[tp->chipset].name,
> -		       dev->base_addr,
> -		       dev->dev_addr[0], dev->dev_addr[1],
> -		       dev->dev_addr[2], dev->dev_addr[3],
> -		       dev->dev_addr[4], dev->dev_addr[5], xid, dev->irq);
> +		       dev->base_addr, dev->dev_addr, xid, dev->irq);
>  	}
>  
>  	rtl8169_init_phy(dev, tp);
> -- 
> 1.7.0.4
> 
> 
> From 882abb3219856eaeef8dbb7f1568616c8f890ec0 Mon Sep 17 00:00:00 2001
> From: David Dillow <dave@thedillows.org>
> Date: Wed, 3 Mar 2010 16:33:10 +0000
> Subject: [PATCH 2/6] r8169: use correct barrier between cacheable and non-cacheable memory
> 
> r8169 needs certain writes to be visible to other CPUs or the NIC before
> touching the hardware, but was using smp_wmb() which is only required to
> order cacheable memory access. Switch to wmb() which is required to
> order both cacheable and non-cacheable memory.
> 
> Noticed by Catalin Marinas and Paul Mackerras.
> 
> Signed-off-by: David Dillow <dave@thedillows.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 4c020a961a812ffae9846b917304cea504c3a733)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

Switches to use wmb().  Looks fine.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 2b1d500..40b7089 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -4299,7 +4299,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>  
>  	tp->cur_tx += frags + 1;
>  
> -	smp_wmb();
> +	wmb();
>  
>  	RTL_W8(TxPoll, NPQ);	/* set polling bit */
>  
> @@ -4659,7 +4659,7 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
>  		 * until it does.
>  		 */
>  		tp->intr_mask = 0xffff;
> -		smp_wmb();
> +		wmb();
>  		RTL_W16(IntrMask, tp->intr_event);
>  	}
>  
> -- 
> 1.7.0.4
> 
> 
> From 785023962471274c52aa6e7a95cc67659fa74085 Mon Sep 17 00:00:00 2001
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Sat, 27 Mar 2010 19:35:46 -0700
> Subject: [PATCH 3/6] r8169: fix broken register writes
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This is quite similar to b39fe41f481d20c201012e4483e76c203802dda7
> though said registers are not even documented as 64-bit registers
> - as opposed to the initial TxDescStartAddress ones - but as single
> bytes which must be combined into 32 bits at the MMIO read/write
> level before being merged into a 64 bit logical entity.
> 
> Credits go to Ben Hutchings <ben@decadent.org.uk> for the MAR
> registers (aka "multicast is broken for ages on ARM) and to
> Timo Teräs <timo.teras@iki.fi> for the MAC registers.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 78f1cd02457252e1ffbc6caa44a17424a45286b8)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

Fixes up write ordering.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 40b7089..fc4632b 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -2827,8 +2827,8 @@ static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
>  	spin_lock_irq(&tp->lock);
>  
>  	RTL_W8(Cfg9346, Cfg9346_Unlock);
> -	RTL_W32(MAC0, low);
>  	RTL_W32(MAC4, high);
> +	RTL_W32(MAC0, low);
>  	RTL_W8(Cfg9346, Cfg9346_Lock);
>  
>  	spin_unlock_irq(&tp->lock);
> @@ -4797,8 +4797,8 @@ static void rtl_set_rx_mode(struct net_device *dev)
>  		mc_filter[1] = swab32(data);
>  	}
>  
> -	RTL_W32(MAR0 + 0, mc_filter[0]);
>  	RTL_W32(MAR0 + 4, mc_filter[1]);
> +	RTL_W32(MAR0 + 0, mc_filter[0]);
>  
>  	RTL_W32(RxConfig, tmp);
>  
> -- 
> 1.7.0.4
> 
> 
> From fa3553f63489d0283f81211871511de4f5245f1e Mon Sep 17 00:00:00 2001
> From: Neil Horman <nhorman@redhat.com>
> Date: Mon, 29 Mar 2010 13:16:02 -0700
> Subject: [PATCH 4/6] r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
> 
> Official patch to fix the r8169 frame length check error.
> 
> Based on this initial thread:
> http://marc.info/?l=linux-netdev&m=126202972828626&w=1
> This is the official patch to fix the frame length problems in the r8169
> driver.  As noted in the previous thread, while this patch incurs a performance
> hit on the driver, its possible to improve performance dynamically by updating
> the mtu and rx_copybreak values at runtime to return performance to what it was
> for those NICS which are unaffected by the ideosyncracy (if there are any).
> 
> Summary:
> 
>     A while back Eric submitted a patch for r8169 in which the proper
> allocated frame size was written to RXMaxSize to prevent the NIC from dmaing too
> much data.  This was done in commit fdd7b4c3302c93f6833e338903ea77245eb510b4.  A
> long time prior to that however, Francois posted
> 126fa4b9ca5d9d7cb7d46f779ad3bd3631ca387c, which expiclitly disabled the MaxSize
> setting due to the fact that the hardware behaved in odd ways when overlong
> frames were received on NIC's supported by this driver.  This was mentioned in a
> security conference recently:
> http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html
> 
> It seems that if we can't enable frame size filtering, then, as Eric correctly
> noticed, we can find ourselves DMA-ing too much data to a buffer, causing
> corruption.  As a result is seems that we are forced to allocate a frame which
> is ready to handle a maximally sized receive.
> 
> This obviously has performance issues with it, so to mitigate that issue, this
> patch does two things:
> 
> 1) Raises the copybreak value to the frame allocation size, which should force
> appropriately sized packets to get allocated on rx, rather than a full new 16k
> buffer.
> 
> 2) This patch only disables frame filtering initially (i.e., during the NIC
> open), changing the MTU results in ring buffer allocation of a size in relation
> to the new mtu (along with a warning indicating that this is dangerous).
> 
> Because of item (2), individuals who can't cope with the performance hit (or can
> otherwise filter frames to prevent the bug), or who have hardware they are sure
> is unaffected by this issue, can manually lower the copybreak and reset the mtu
> such that performance is restored easily.
> 
> Signed-off-by: Neil Horman <nhorman@redhat.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit c0cd884af045338476b8e69a61fceb3f34ff22f1)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

I believe we are already planning on pulling this patch in for the
upcoming round of CVE's.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |   29 ++++++++++++++++++++++++-----
>  1 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index fc4632b..8a044d0 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -186,7 +186,12 @@ static struct pci_device_id rtl8169_pci_tbl[] = {
>  
>  MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
>  
> -static int rx_copybreak = 200;
> +/*
> + * we set our copybreak very high so that we don't have
> + * to allocate 16k frames all the time (see note in
> + * rtl8169_open()
> + */
> +static int rx_copybreak = 16383;
>  static int use_dac;
>  static struct {
>  	u32 msg_enable;
> @@ -3247,9 +3252,13 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
>  }
>  
>  static void rtl8169_set_rxbufsize(struct rtl8169_private *tp,
> -				  struct net_device *dev)
> +				  unsigned int mtu)
>  {
> -	unsigned int max_frame = dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +	unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +
> +	if (max_frame != 16383)
> +		printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
> +			"May lead to frame reception errors!\n");
>  
>  	tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE;
>  }
> @@ -3261,7 +3270,17 @@ static int rtl8169_open(struct net_device *dev)
>  	int retval = -ENOMEM;
>  
> 
> -	rtl8169_set_rxbufsize(tp, dev);
> +	/*
> +	 * Note that we use a magic value here, its wierd I know
> +	 * its done because, some subset of rtl8169 hardware suffers from
> +	 * a problem in which frames received that are longer than
> +	 * the size set in RxMaxSize register return garbage sizes
> +	 * when received.  To avoid this we need to turn off filtering,
> +	 * which is done by setting a value of 16383 in the RxMaxSize register
> +	 * and allocating 16k frames to handle the largest possible rx value
> +	 * thats what the magic math below does.
> +	 */
> +	rtl8169_set_rxbufsize(tp, 16383 - VLAN_ETH_HLEN - ETH_FCS_LEN);
>  
>  	/*
>  	 * Rx and Tx desscriptors needs 256 bytes alignment.
> @@ -3914,7 +3933,7 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
>  
>  	rtl8169_down(dev);
>  
> -	rtl8169_set_rxbufsize(tp, dev);
> +	rtl8169_set_rxbufsize(tp, dev->mtu);
>  
>  	ret = rtl8169_init_ring(dev);
>  	if (ret < 0)
> -- 
> 1.7.0.4
> 
> 
> From 49a8d5a48f9ee7dcca41deac8a631a6819246bba Mon Sep 17 00:00:00 2001
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Thu, 1 Apr 2010 07:30:07 +0000
> Subject: [PATCH 5/6] r8169: clean up my printk uglyness
> 
> Fix formatting on r8169 printk
> 
> Brandon Philips noted that I had a spacing issue in my printk for the
> last r8169 patch that made it quite ugly.  Fix that up and add the PFX
> macro to it as well so it looks like the other r8169 printks
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 93f4d91d879acfcb0ba9c2725e3133fcff2dfd1e)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

Small printk fix up.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 8a044d0..151abcb 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -3257,8 +3257,8 @@ static void rtl8169_set_rxbufsize(struct rtl8169_private *tp,
>  	unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
>  
>  	if (max_frame != 16383)
> -		printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
> -			"May lead to frame reception errors!\n");
> +		printk(KERN_WARNING PFX "WARNING! Changing of MTU on this "
> +			"NIC may lead to frame reception errors!\n");
>  
>  	tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE;
>  }
> -- 
> 1.7.0.4
> 
> 
> From 9022be937f0796b9516c611e0930d592aaf6b4fb Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?fran=C3=A7ois=20romieu?= <romieu@fr.zoreil.com>
> Date: Mon, 26 Apr 2010 11:42:58 +0000
> Subject: [PATCH 6/6] r8169: more broken register writes workaround
> 
> 78f1cd02457252e1ffbc6caa44a17424a45286b8 ("fix broken register writes")
> does not work for Al Viro's r8169 (XID 18000000).
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 908ba2bfd22253f26fa910cd855e4ccffb1467d0)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

More register write fixes.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/net/r8169.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 151abcb..78d43ca 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -2832,8 +2832,13 @@ static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
>  	spin_lock_irq(&tp->lock);
>  
>  	RTL_W8(Cfg9346, Cfg9346_Unlock);
> +
>  	RTL_W32(MAC4, high);
> +	RTL_R32(MAC4);
> +
>  	RTL_W32(MAC0, low);
> +	RTL_R32(MAC0);
> +
>  	RTL_W8(Cfg9346, Cfg9346_Lock);
>  
>  	spin_unlock_irq(&tp->lock);
Stefan Bader - May 19, 2010, 6:31 p.m.
Somehow a lot of these feel too familiar. I would wait on those until I managed
to pull in the latest stable patches and then get back to those.

-Stefan

On 05/14/2010 11:35 AM, Tim Gardner wrote:
> The following series of changes are clean cherry picks from upstream sufficient
> and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742
> 
> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>   Chase Douglas (1):
>         UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742
> 
> David Dillow (1):
>       r8169: use correct barrier between cacheable and non-cacheable memory
> 
> Francois Romieu (1):
>       r8169: fix broken register writes
> 
> H Hartley Sweeten (1):
>       drivers/net/r8169.c: use %pM to shown MAC address
> 
> Neil Horman (2):
>       r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
>       r8169: clean up my printk uglyness
> 
> françois romieu (1):
>       r8169: more broken register writes workaround
> 
>  drivers/net/r8169.c |   51 +++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 35 insertions(+), 16 deletions(-)
> From a6c647a849c6dc21fb6c1a1bd83e791ad77d87a1 Mon Sep 17 00:00:00 2001
> From: H Hartley Sweeten <hsweeten@visionengravers.com>
> Date: Tue, 29 Dec 2009 20:10:01 -0800
> Subject: [PATCH 1/6] drivers/net/r8169.c: use %pM to shown MAC address
> 
> Use the %pM kernel extension to display the MAC address.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 30a6ae8d477dc90254eb785d8ccff6dfe7d9082e)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/net/r8169.c |    9 ++-------
>  1 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 7b98624..2b1d500 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -3195,15 +3195,10 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (netif_msg_probe(tp)) {
>  		u32 xid = RTL_R32(TxConfig) & 0x9cf0f8ff;
>  
> -		printk(KERN_INFO "%s: %s at 0x%lx, "
> -		       "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x, "
> -		       "XID %08x IRQ %d\n",
> +		printk(KERN_INFO "%s: %s at 0x%lx, %pM, XID %08x IRQ %d\n",
>  		       dev->name,
>  		       rtl_chip_info[tp->chipset].name,
> -		       dev->base_addr,
> -		       dev->dev_addr[0], dev->dev_addr[1],
> -		       dev->dev_addr[2], dev->dev_addr[3],
> -		       dev->dev_addr[4], dev->dev_addr[5], xid, dev->irq);
> +		       dev->base_addr, dev->dev_addr, xid, dev->irq);
>  	}
>  
>  	rtl8169_init_phy(dev, tp);
Tim Gardner - May 19, 2010, 6:38 p.m.
Indeed, all of these patches are now in 2.6.32.y, but they were not when 
I first proposed them. They likely appeared in stable 'cause I sent an 
email requesting that they be added.

rtg

On 05/19/2010 12:31 PM, Stefan Bader wrote:
> Somehow a lot of these feel too familiar. I would wait on those until I managed
> to pull in the latest stable patches and then get back to those.
>
> -Stefan
>
> On 05/14/2010 11:35 AM, Tim Gardner wrote:
>> The following series of changes are clean cherry picks from upstream sufficient
>> and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742
>>
>> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>>    Chase Douglas (1):
>>          UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742
>>
>> David Dillow (1):
>>        r8169: use correct barrier between cacheable and non-cacheable memory
>>
>> Francois Romieu (1):
>>        r8169: fix broken register writes
>>
>> H Hartley Sweeten (1):
>>        drivers/net/r8169.c: use %pM to shown MAC address
>>
>> Neil Horman (2):
>>        r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
>>        r8169: clean up my printk uglyness
>>
>> françois romieu (1):
>>        r8169: more broken register writes workaround
>>
>>   drivers/net/r8169.c |   51 +++++++++++++++++++++++++++++++++++----------------
>>   1 files changed, 35 insertions(+), 16 deletions(-)
>>  From a6c647a849c6dc21fb6c1a1bd83e791ad77d87a1 Mon Sep 17 00:00:00 2001
>> From: H Hartley Sweeten<hsweeten@visionengravers.com>
>> Date: Tue, 29 Dec 2009 20:10:01 -0800
>> Subject: [PATCH 1/6] drivers/net/r8169.c: use %pM to shown MAC address
>>
>> Use the %pM kernel extension to display the MAC address.
>>
>> Signed-off-by: H Hartley Sweeten<hsweeten@visionengravers.com>
>> Signed-off-by: David S. Miller<davem@davemloft.net>
>> (cherry picked from commit 30a6ae8d477dc90254eb785d8ccff6dfe7d9082e)
>>
>> Signed-off-by: Tim Gardner<tim.gardner@canonical.com>
>> ---
>>   drivers/net/r8169.c |    9 ++-------
>>   1 files changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
>> index 7b98624..2b1d500 100644
>> --- a/drivers/net/r8169.c
>> +++ b/drivers/net/r8169.c
>> @@ -3195,15 +3195,10 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   	if (netif_msg_probe(tp)) {
>>   		u32 xid = RTL_R32(TxConfig)&  0x9cf0f8ff;
>>
>> -		printk(KERN_INFO "%s: %s at 0x%lx, "
>> -		       "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x, "
>> -		       "XID %08x IRQ %d\n",
>> +		printk(KERN_INFO "%s: %s at 0x%lx, %pM, XID %08x IRQ %d\n",
>>   		       dev->name,
>>   		       rtl_chip_info[tp->chipset].name,
>> -		       dev->base_addr,
>> -		       dev->dev_addr[0], dev->dev_addr[1],
>> -		       dev->dev_addr[2], dev->dev_addr[3],
>> -		       dev->dev_addr[4], dev->dev_addr[5], xid, dev->irq);
>> +		       dev->base_addr, dev->dev_addr, xid, dev->irq);
>>   	}
>>
>>   	rtl8169_init_phy(dev, tp);
>
>
>
Stefan Bader - May 20, 2010, 5:58 p.m.
On 05/14/2010 11:35 AM, Tim Gardner wrote:
> The following series of changes are clean cherry picks from upstream sufficient
> and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742
> 
> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>   Chase Douglas (1):
>         UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742
> 
> David Dillow (1):
>       r8169: use correct barrier between cacheable and non-cacheable memory
2.6.32.13

> 
> Francois Romieu (1):
>       r8169: fix broken register writes
2.6.32.13

> 
> H Hartley Sweeten (1):
>       drivers/net/r8169.c: use %pM to shown MAC address
This one I did not find yet in stable. Looks reasonably easy and probably is not
deemed worthy for fixing a problem. Have you sent that as well and got any
feedback? It looks like only being cleanup, is it necessary?

> 
> Neil Horman (2):
>       r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)
2.6.32.12 (and will be included in next security upload)

>       r8169: clean up my printk uglyness
2.6.32.12

> 
> françois romieu (1):
>       r8169: more broken register writes workaround
2.6.32.13

> 
>  drivers/net/r8169.c |   51 +++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 35 insertions(+), 16 deletions(-)
> From a6c647a849c6dc21fb6c1a1bd83e791ad77d87a1 Mon Sep 17 00:00:00 2001
> From: H Hartley Sweeten <hsweeten@visionengravers.com>
> Date: Tue, 29 Dec 2009 20:10:01 -0800
> Subject: [PATCH 1/6] drivers/net/r8169.c: use %pM to shown MAC address
> 
> Use the %pM kernel extension to display the MAC address.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 30a6ae8d477dc90254eb785d8ccff6dfe7d9082e)
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/net/r8169.c |    9 ++-------
>  1 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 7b98624..2b1d500 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -3195,15 +3195,10 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (netif_msg_probe(tp)) {
>  		u32 xid = RTL_R32(TxConfig) & 0x9cf0f8ff;
>  
> -		printk(KERN_INFO "%s: %s at 0x%lx, "
> -		       "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x, "
> -		       "XID %08x IRQ %d\n",
> +		printk(KERN_INFO "%s: %s at 0x%lx, %pM, XID %08x IRQ %d\n",
>  		       dev->name,
>  		       rtl_chip_info[tp->chipset].name,
> -		       dev->base_addr,
> -		       dev->dev_addr[0], dev->dev_addr[1],
> -		       dev->dev_addr[2], dev->dev_addr[3],
> -		       dev->dev_addr[4], dev->dev_addr[5], xid, dev->irq);
> +		       dev->base_addr, dev->dev_addr, xid, dev->irq);
>  	}
>  
>  	rtl8169_init_phy(dev, tp);
Tim Gardner - May 20, 2010, 6:06 p.m.
On 05/20/2010 11:58 AM, Stefan Bader wrote:
> On 05/14/2010 11:35 AM, Tim Gardner wrote:
>> The following series of changes are clean cherry picks from upstream sufficient
>> and necessary to correct the bug reported in https://bugs.launchpad.net/bugs/562742
>>
>> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>>    Chase Douglas (1):
>>          UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-lucid.git lp562742
>>
>> David Dillow (1):
>>        r8169: use correct barrier between cacheable and non-cacheable memory
> 2.6.32.13
>
>>
>> Francois Romieu (1):
>>        r8169: fix broken register writes
> 2.6.32.13
>
>>
>> H Hartley Sweeten (1):
>>        drivers/net/r8169.c: use %pM to shown MAC address
> This one I did not find yet in stable. Looks reasonably easy and probably is not
> deemed worthy for fixing a problem. Have you sent that as well and got any
> feedback? It looks like only being cleanup, is it necessary?
>

I thought it was required so that subsequent patches would apply. If its 
not, then I wouldn't bother.

rtg

Patch

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 7b98624..2b1d500 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3195,15 +3195,10 @@  rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (netif_msg_probe(tp)) {
 		u32 xid = RTL_R32(TxConfig) & 0x9cf0f8ff;
 
-		printk(KERN_INFO "%s: %s at 0x%lx, "
-		       "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x, "
-		       "XID %08x IRQ %d\n",
+		printk(KERN_INFO "%s: %s at 0x%lx, %pM, XID %08x IRQ %d\n",
 		       dev->name,
 		       rtl_chip_info[tp->chipset].name,
-		       dev->base_addr,
-		       dev->dev_addr[0], dev->dev_addr[1],
-		       dev->dev_addr[2], dev->dev_addr[3],
-		       dev->dev_addr[4], dev->dev_addr[5], xid, dev->irq);
+		       dev->base_addr, dev->dev_addr, xid, dev->irq);
 	}
 
 	rtl8169_init_phy(dev, tp);
-- 
1.7.0.4


From 882abb3219856eaeef8dbb7f1568616c8f890ec0 Mon Sep 17 00:00:00 2001
From: David Dillow <dave@thedillows.org>
Date: Wed, 3 Mar 2010 16:33:10 +0000
Subject: [PATCH 2/6] r8169: use correct barrier between cacheable and non-cacheable memory

r8169 needs certain writes to be visible to other CPUs or the NIC before
touching the hardware, but was using smp_wmb() which is only required to
order cacheable memory access. Switch to wmb() which is required to
order both cacheable and non-cacheable memory.

Noticed by Catalin Marinas and Paul Mackerras.

Signed-off-by: David Dillow <dave@thedillows.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 4c020a961a812ffae9846b917304cea504c3a733)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 2b1d500..40b7089 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4299,7 +4299,7 @@  static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	tp->cur_tx += frags + 1;
 
-	smp_wmb();
+	wmb();
 
 	RTL_W8(TxPoll, NPQ);	/* set polling bit */
 
@@ -4659,7 +4659,7 @@  static int rtl8169_poll(struct napi_struct *napi, int budget)
 		 * until it does.
 		 */
 		tp->intr_mask = 0xffff;
-		smp_wmb();
+		wmb();
 		RTL_W16(IntrMask, tp->intr_event);
 	}
 
-- 
1.7.0.4


From 785023962471274c52aa6e7a95cc67659fa74085 Mon Sep 17 00:00:00 2001
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 27 Mar 2010 19:35:46 -0700
Subject: [PATCH 3/6] r8169: fix broken register writes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is quite similar to b39fe41f481d20c201012e4483e76c203802dda7
though said registers are not even documented as 64-bit registers
- as opposed to the initial TxDescStartAddress ones - but as single
bytes which must be combined into 32 bits at the MMIO read/write
level before being merged into a 64 bit logical entity.

Credits go to Ben Hutchings <ben@decadent.org.uk> for the MAR
registers (aka "multicast is broken for ages on ARM) and to
Timo Teräs <timo.teras@iki.fi> for the MAC registers.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 78f1cd02457252e1ffbc6caa44a17424a45286b8)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 40b7089..fc4632b 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2827,8 +2827,8 @@  static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 	spin_lock_irq(&tp->lock);
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
-	RTL_W32(MAC0, low);
 	RTL_W32(MAC4, high);
+	RTL_W32(MAC0, low);
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 
 	spin_unlock_irq(&tp->lock);
@@ -4797,8 +4797,8 @@  static void rtl_set_rx_mode(struct net_device *dev)
 		mc_filter[1] = swab32(data);
 	}
 
-	RTL_W32(MAR0 + 0, mc_filter[0]);
 	RTL_W32(MAR0 + 4, mc_filter[1]);
+	RTL_W32(MAR0 + 0, mc_filter[0]);
 
 	RTL_W32(RxConfig, tmp);
 
-- 
1.7.0.4


From fa3553f63489d0283f81211871511de4f5245f1e Mon Sep 17 00:00:00 2001
From: Neil Horman <nhorman@redhat.com>
Date: Mon, 29 Mar 2010 13:16:02 -0700
Subject: [PATCH 4/6] r8169: offical fix for CVE-2009-4537 (overlength frame DMAs)

Official patch to fix the r8169 frame length check error.

Based on this initial thread:
http://marc.info/?l=linux-netdev&m=126202972828626&w=1
This is the official patch to fix the frame length problems in the r8169
driver.  As noted in the previous thread, while this patch incurs a performance
hit on the driver, its possible to improve performance dynamically by updating
the mtu and rx_copybreak values at runtime to return performance to what it was
for those NICS which are unaffected by the ideosyncracy (if there are any).

Summary:

    A while back Eric submitted a patch for r8169 in which the proper
allocated frame size was written to RXMaxSize to prevent the NIC from dmaing too
much data.  This was done in commit fdd7b4c3302c93f6833e338903ea77245eb510b4.  A
long time prior to that however, Francois posted
126fa4b9ca5d9d7cb7d46f779ad3bd3631ca387c, which expiclitly disabled the MaxSize
setting due to the fact that the hardware behaved in odd ways when overlong
frames were received on NIC's supported by this driver.  This was mentioned in a
security conference recently:
http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html

It seems that if we can't enable frame size filtering, then, as Eric correctly
noticed, we can find ourselves DMA-ing too much data to a buffer, causing
corruption.  As a result is seems that we are forced to allocate a frame which
is ready to handle a maximally sized receive.

This obviously has performance issues with it, so to mitigate that issue, this
patch does two things:

1) Raises the copybreak value to the frame allocation size, which should force
appropriately sized packets to get allocated on rx, rather than a full new 16k
buffer.

2) This patch only disables frame filtering initially (i.e., during the NIC
open), changing the MTU results in ring buffer allocation of a size in relation
to the new mtu (along with a warning indicating that this is dangerous).

Because of item (2), individuals who can't cope with the performance hit (or can
otherwise filter frames to prevent the bug), or who have hardware they are sure
is unaffected by this issue, can manually lower the copybreak and reset the mtu
such that performance is restored easily.

Signed-off-by: Neil Horman <nhorman@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit c0cd884af045338476b8e69a61fceb3f34ff22f1)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |   29 ++++++++++++++++++++++++-----
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index fc4632b..8a044d0 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -186,7 +186,12 @@  static struct pci_device_id rtl8169_pci_tbl[] = {
 
 MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
 
-static int rx_copybreak = 200;
+/*
+ * we set our copybreak very high so that we don't have
+ * to allocate 16k frames all the time (see note in
+ * rtl8169_open()
+ */
+static int rx_copybreak = 16383;
 static int use_dac;
 static struct {
 	u32 msg_enable;
@@ -3247,9 +3252,13 @@  static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
 }
 
 static void rtl8169_set_rxbufsize(struct rtl8169_private *tp,
-				  struct net_device *dev)
+				  unsigned int mtu)
 {
-	unsigned int max_frame = dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+	unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+
+	if (max_frame != 16383)
+		printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
+			"May lead to frame reception errors!\n");
 
 	tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE;
 }
@@ -3261,7 +3270,17 @@  static int rtl8169_open(struct net_device *dev)
 	int retval = -ENOMEM;
 
 
-	rtl8169_set_rxbufsize(tp, dev);
+	/*
+	 * Note that we use a magic value here, its wierd I know
+	 * its done because, some subset of rtl8169 hardware suffers from
+	 * a problem in which frames received that are longer than
+	 * the size set in RxMaxSize register return garbage sizes
+	 * when received.  To avoid this we need to turn off filtering,
+	 * which is done by setting a value of 16383 in the RxMaxSize register
+	 * and allocating 16k frames to handle the largest possible rx value
+	 * thats what the magic math below does.
+	 */
+	rtl8169_set_rxbufsize(tp, 16383 - VLAN_ETH_HLEN - ETH_FCS_LEN);
 
 	/*
 	 * Rx and Tx desscriptors needs 256 bytes alignment.
@@ -3914,7 +3933,7 @@  static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
 
 	rtl8169_down(dev);
 
-	rtl8169_set_rxbufsize(tp, dev);
+	rtl8169_set_rxbufsize(tp, dev->mtu);
 
 	ret = rtl8169_init_ring(dev);
 	if (ret < 0)
-- 
1.7.0.4


From 49a8d5a48f9ee7dcca41deac8a631a6819246bba Mon Sep 17 00:00:00 2001
From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 1 Apr 2010 07:30:07 +0000
Subject: [PATCH 5/6] r8169: clean up my printk uglyness

Fix formatting on r8169 printk

Brandon Philips noted that I had a spacing issue in my printk for the
last r8169 patch that made it quite ugly.  Fix that up and add the PFX
macro to it as well so it looks like the other r8169 printks

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 93f4d91d879acfcb0ba9c2725e3133fcff2dfd1e)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 8a044d0..151abcb 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3257,8 +3257,8 @@  static void rtl8169_set_rxbufsize(struct rtl8169_private *tp,
 	unsigned int max_frame = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
 
 	if (max_frame != 16383)
-		printk(KERN_WARNING "WARNING! Changing of MTU on this NIC"
-			"May lead to frame reception errors!\n");
+		printk(KERN_WARNING PFX "WARNING! Changing of MTU on this "
+			"NIC may lead to frame reception errors!\n");
 
 	tp->rx_buf_sz = (max_frame > RX_BUF_SIZE) ? max_frame : RX_BUF_SIZE;
 }
-- 
1.7.0.4


From 9022be937f0796b9516c611e0930d592aaf6b4fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?fran=C3=A7ois=20romieu?= <romieu@fr.zoreil.com>
Date: Mon, 26 Apr 2010 11:42:58 +0000
Subject: [PATCH 6/6] r8169: more broken register writes workaround

78f1cd02457252e1ffbc6caa44a17424a45286b8 ("fix broken register writes")
does not work for Al Viro's r8169 (XID 18000000).

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 908ba2bfd22253f26fa910cd855e4ccffb1467d0)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/net/r8169.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 151abcb..78d43ca 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2832,8 +2832,13 @@  static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 	spin_lock_irq(&tp->lock);
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
+
 	RTL_W32(MAC4, high);
+	RTL_R32(MAC4);
+
 	RTL_W32(MAC0, low);
+	RTL_R32(MAC0);
+
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 
 	spin_unlock_irq(&tp->lock);