Patchwork e1000: Discard oversized packets based on SBP|LPE

login
register
mail settings
Submitter Michael Contreras
Date Dec. 5, 2012, 6:31 p.m.
Message ID <20121205183130.GA26052@inetric.com>
Download mbox | patch
Permalink /patch/203921/
State New
Headers show

Comments

Michael Contreras - Dec. 5, 2012, 6:31 p.m.
Discard packets longer than 16384 when !SBP to match the hardware behavior.

Signed-off-by: Michael Contreras <michael@inetric.com>
---
 hw/e1000.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Stefan Hajnoczi - Dec. 18, 2012, 1:44 p.m.
On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
> Discard packets longer than 16384 when !SBP to match the hardware behavior.
> 
> Signed-off-by: Michael Contreras <michael@inetric.com>
> ---
>  hw/e1000.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Thanks, applied to the net tree:
https://github.com/stefanha/qemu/commits/net

Stefan
Michael Tokarev - Dec. 18, 2012, 4:20 p.m.
On 18.12.2012 17:44, Stefan Hajnoczi wrote:
> On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
>> Discard packets longer than 16384 when !SBP to match the hardware behavior.
>>
>> Signed-off-by: Michael Contreras <michael@inetric.com>
>> ---
>>  hw/e1000.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)

It looks like another very good candidate for -stable (up to quite some
releases of qemu ago), together with the previous similar patch.

Isn't it quite a bit security-sensitive too?

Thanks,

/mjt
Stefan Hajnoczi - Dec. 18, 2012, 4:49 p.m.
On Tue, Dec 18, 2012 at 5:20 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> On 18.12.2012 17:44, Stefan Hajnoczi wrote:
>> On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
>>> Discard packets longer than 16384 when !SBP to match the hardware behavior.
>>>
>>> Signed-off-by: Michael Contreras <michael@inetric.com>
>>> ---
>>>  hw/e1000.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> It looks like another very good candidate for -stable (up to quite some
> releases of qemu ago), together with the previous similar patch.

Yes, it's good for -stable.

Stefan
Michael Contreras - Dec. 18, 2012, 5:34 p.m.
On Tue, Dec 18, 2012 at 05:49:16PM +0100, Stefan Hajnoczi wrote:
> On Tue, Dec 18, 2012 at 5:20 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> > On 18.12.2012 17:44, Stefan Hajnoczi wrote:
> >> On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
> >>> Discard packets longer than 16384 when !SBP to match the hardware behavior.
> >>>
> >>> Signed-off-by: Michael Contreras <michael@inetric.com>
> >>> ---
> >>>  hw/e1000.c | 7 +++++--
> >>>  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > It looks like another very good candidate for -stable (up to quite some
> > releases of qemu ago), together with the previous similar patch.
> 
> Yes, it's good for -stable.
> 
> Stefan

Thanks guys. Any update on the CVE number? Seems the KVM qemu git tree
still has this vulnerability. Xen has the fix in their qemu unstable
git mirror, but hasn't applied it yet either.

Michael
Stefan Hajnoczi - Dec. 19, 2012, 11:42 a.m.
On Tue, Dec 18, 2012 at 12:34:22PM -0500, Michael Contreras wrote:
> On Tue, Dec 18, 2012 at 05:49:16PM +0100, Stefan Hajnoczi wrote:
> > On Tue, Dec 18, 2012 at 5:20 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> > > On 18.12.2012 17:44, Stefan Hajnoczi wrote:
> > >> On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
> > >>> Discard packets longer than 16384 when !SBP to match the hardware behavior.
> > >>>
> > >>> Signed-off-by: Michael Contreras <michael@inetric.com>
> > >>> ---
> > >>>  hw/e1000.c | 7 +++++--
> > >>>  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > It looks like another very good candidate for -stable (up to quite some
> > > releases of qemu ago), together with the previous similar patch.
> > 
> > Yes, it's good for -stable.
> > 
> > Stefan
> 
> Thanks guys. Any update on the CVE number? Seems the KVM qemu git tree
> still has this vulnerability. Xen has the fix in their qemu unstable
> git mirror, but hasn't applied it yet either.

Your original LPE patch went into QEMU 1.3.  qemu-kvm.git is no longer
relevant - it has been merged back into qemu.git and has therefore not
been updated since October 11.  Use qemu.git.

Perhaps others can provide info on the CVE and Xen.

Stefan
Michael Tokarev - Dec. 30, 2012, 8:29 a.m.
18.12.2012 21:34, Michael Contreras пишет:
> On Tue, Dec 18, 2012 at 05:49:16PM +0100, Stefan Hajnoczi wrote:
>> On Tue, Dec 18, 2012 at 5:20 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>>> On 18.12.2012 17:44, Stefan Hajnoczi wrote:
>>>> On Wed, Dec 05, 2012 at 01:31:30PM -0500, Michael Contreras wrote:
>>>>> Discard packets longer than 16384 when !SBP to match the hardware behavior.
>>>>>
>>>>> Signed-off-by: Michael Contreras <michael@inetric.com>
>>>>> ---
>>>>>   hw/e1000.c | 7 +++++--
>>>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> It looks like another very good candidate for -stable (up to quite some
>>> releases of qemu ago), together with the previous similar patch.
>>
>> Yes, it's good for -stable.
>>
>> Stefan
>
> Thanks guys. Any update on the CVE number? Seems the KVM qemu git tree
> still has this vulnerability. Xen has the fix in their qemu unstable
> git mirror, but hasn't applied it yet either.

This issue has been assigned CVE-2012-6075.

qemu-kvm does not exist anymore, it is just an internal development
tree for qemu, sort of like a subsystem tree - there will be no
more qemu-kvm releases.

So we care only about qemu (main, older versions, incl. 0.12 and 0.15,
are also affected), old qemu-kvm, and xen.  CC'ing afaerber for 0.15.

Thank you!

/mjt

Patch

diff --git a/hw/e1000.c b/hw/e1000.c
index 5537ad2..e772c8e 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -61,6 +61,8 @@  static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
 
 /* this is the size past which hardware will drop packets when setting LPE=0 */
 #define MAXIMUM_ETHERNET_VLAN_SIZE 1522
+/* this is the size past which hardware will drop packets when setting LPE=1 */
+#define MAXIMUM_ETHERNET_LPE_SIZE 16384
 
 /*
  * HW models:
@@ -809,8 +811,9 @@  e1000_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     }
 
     /* Discard oversized packets if !LPE and !SBP. */
-    if (size > MAXIMUM_ETHERNET_VLAN_SIZE
-        && !(s->mac_reg[RCTL] & E1000_RCTL_LPE)
+    if ((size > MAXIMUM_ETHERNET_LPE_SIZE ||
+        (size > MAXIMUM_ETHERNET_VLAN_SIZE
+        && !(s->mac_reg[RCTL] & E1000_RCTL_LPE)))
         && !(s->mac_reg[RCTL] & E1000_RCTL_SBP)) {
         return size;
     }