netdev - Re: [PATCH net-next-2.6] e1000: don't enable dma receives until after dma address has been setup

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1316138635.3661.55.camel@jtkirshe-mobl>
Date:	Thu, 15 Sep 2011 19:03:54 -0700
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Dean Nelson <dnelson@...hat.com>
Cc:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	"Dave, Tushar N" <tushar.n.dave@...el.com>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>
Subject: Re: [PATCH net-next-2.6] e1000: don't enable dma receives until
 after dma address has been setup

On Thu, 2011-09-15 at 11:22 -0700, Dean Nelson wrote:
> On 09/15/2011 12:21 PM, Jesse Brandeburg wrote:
> > On Wed, 14 Sep 2011 17:31:38 -0700
> > Dean Nelson<dnelson@...hat.com>  wrote:
> >
> >> Doing an 'ifconfig ethN down' followed by an 'ifconfig ethN up' on a
> >> qemu-kvm guest system configured with two e1000 NICs can result in an
> >> 'unable to handle kernel paging request at 0000000100000000' or 'bad
> >> page map in process ...' or something similar.
> >
> > <snip>
> >
> >> The corruption appears to result from the following...
> >>
> <snip>
> >>
> >> I realize that in the guest, we're dealing with an e1000 NIC that is
> >> software emulated by qemu-kvm. The problem doesn't appear to occur on
> >> bare-metal. Andy suspects that this is because in the emulator
> >> link-up is essentially instant and traffic can start flowing
> >> immediately. Whereas on bare-metal, link-up usually seems to take at
> >> least a few milliseconds. And this might be enough to prevent traffic
> >> from flowing into the device inside the window where E1000_RCTL_EN is
> >> set.
> >
> > nice analysis dean, yes, we shouldn't enable rx before we have the
> > hardware all ready.
> 
> Thank you.
> 
> 
> > You didn't mention however that the hardware is reset in e1000_down,
> > which will clear the RDBAL/RDBAH in real hardware.
> 
> You are correct, I did fail to mention the reset. And the clearing of
> RDBAL/RDHAH was definitely not happening in the qemu-kvm emulator.
> 
> 
> >> So perhaps a modification needs to be made to the qemu-kvm e1000 NIC
> >> emulator to delay the link-up. But in defense of the emulator, it
> >> seems like a bad idea to enable dma operations before the address of
> >> the memory to be involved has been made known.
> >
> > the hardware reset code in kvm should also reset to default many
> > registers (almost all of them in fact) which may also end up solving
> > the problem.
> 
> Agreed.
> 
> 
> >> The following patch no longer enables receives in e1000_setup_rctl()
> >> but leaves them however they were. It only enables receives in
> >> e1000_configure_rx(), and only after the dma address has been made
> >> known to the hardware.
> >
> > I still like your patch better as it is more correct.  We could also
> > correct the kvm virtual hardware driver.
> 
> The hardware emulator should definitely be doing a proper hardware reset.
> 
> 
> >> There are two places where e1000_setup_rctl() gets called. The one in
> <snip>
> >>
> >> The e1000e looks to have the same issue. I don't know about igb. But
> >> I'm not aware of either having hardware emulation in qemu-kvm. So
> >> unless this issue is reproducible on bare-metal... it's probably not
> >> a big deal for them.
> >>
> <snip>
> >
> > generally i like the patch.  We should take it in and test it, and I
> > don't really see any problems with it.
> 
> Thanks.
> 
> As mentioned above, the e1000e has a similar algorithm, but the
> FLAG2_NO_DISABLE_RX complicates it a bit. I have no idea what happens
> if receives are enabled while setting RDBAL and RDBAH. Is there any
> possibility that the hardware could try to make use of a half-baked
> address?
> 
> Thanks much for your review of the patch.
> 
> Dean
> 

Thanks Dean!  I will add your patch to my queue.  I will work with Bruce
to review e1000e and get a patch put together.

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)