linux-kernel - Re: [2.6.36-rc5] r8169 regression...

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] [day] [month] [year] [list]

Date:	Wed, 20 Oct 2010 00:07:24 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Daniel J Blueman <daniel.blueman@...il.com>
Cc:	Matthew Garrett <mjg@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux Networking <linux-net@...r.kernel.org>
Subject: Re: [2.6.36-rc5] r8169 regression...

Daniel J Blueman <daniel.blueman@...il.com> :
> Since 2.6.36-rc5, I've been seeing unexpected link up and (as a
> side-effect) pending softirq messages [1] when transferring data over
> a r8169 adapter ("RTL8168d/8111d").
>
> The recent change [2] to make the r8169 driver sensitive to receive
> overflow events is clearly unnecessary for this MAC revision (since it
> was fine without before), so it may be worthwhile to reintroduce the
> 'tp->mac_version == RTL_GIGA_MAC_VER_11' condition on resetting the
> link, tweaked to the problematic MAC version in the Thinkpad X100e.

Matthew, can you apply the patch below and send your XID line ?

At this stage in -rc, I'd rather revert [2], live with some Thinkpad
X100e crashes when under network load while the screen turns off and
avoid any new pesky regression. David ?

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 992db2f..15b8e57 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3208,7 +3208,7 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netif_info(tp, probe, dev, "%s at 0x%lx, %pM, XID %08x IRQ %d\n",
 		   rtl_chip_info[tp->chipset].name,
 		   dev->base_addr, dev->dev_addr,
-		   (u32)(RTL_R32(TxConfig) & 0x9cf0f8ff), dev->irq);
+		   (u32)(RTL_R32(TxConfig) & 0xfcf0f8ff), dev->irq);
 
 	rtl8169_init_phy(dev, tp);
 
[...]
> As a second bonus, we find a latent softirq. When a receive overflow
> is now detected, we get the callchain
> rtl8169_tx_timeout->rtl8169_schedule_work which causes
> rtl8169_reset_task->rtl8169_rx_interrupt->netif_rx from outside the
> interrupt handler. I could cook a patch which would ensure
> netif_rx_ni() is called outside the interrupt handler, but do we care?

You can remove rtl8169_rx_interrupt in rtl8169_reset_task so that
rtl8169_rx_interrupt be only called from softirq context. It will
save some what-is-the-current-context code and avoid a circular
rtl8169_rx_interrupt <-> rtl8169_reset_task dependency.

[...]
> --- [2]
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=801e147cde02f04b5c2f42764cd43a89fc7400a2

-- 
Ueimor
--
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/