| Message ID | 20100120001511.25175.55030.stgit@localhost.localdomain |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Date: Tue, 19 Jan 2010 16:15:38 -0800 > From: Jesse Brandeburg <jesse.brandeburg@intel.com> > > Originally From: Neil Horman <nhorman@tuxdriver.com> > Modified by: Jesse Brandeburg <jesse.brandeburg@intel.com> > > Hey all- > A security discussion was recently given: > http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html > And a patch that I submitted awhile back was brought up. Apparently some of > their testing revealed that they were able to force a buffer fragment in e1000 > in which the trailing fragment was greater than 4 bytes. As a result the > fragment check I introduced failed to detect the fragement and a partial > invalid frame was passed up into the network stack. I've written this patch > to correct it. I'm in the process of testing it now, but it makes good > logical sense to me. Effectively it maintains a per-adapter state variable > which detects a non-EOP frame, and discards it and subsequent non-EOP frames > leading up to _and_ _including_ the next positive-EOP frame (as it is by > definition the last fragment). This should prevent any and all partial frames > from entering the network stack from e1000. > > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Acked-by: Neil Horman <nhorman@tuxdriver.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h index 2a567df..e8932db 100644 --- a/drivers/net/e1000/e1000.h +++ b/drivers/net/e1000/e1000.h @@ -326,6 +326,8 @@ struct e1000_adapter { /* for ioport free */ int bars; int need_ioport; + + bool discarding; }; enum e1000_state_t { diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c index 7e855f9..9bc9fcd 100644 --- a/drivers/net/e1000/e1000_main.c +++ b/drivers/net/e1000/e1000_main.c @@ -3850,13 +3850,22 @@ static bool e1000_clean_rx_irq(struct e1000_adapter *adapter, length = le16_to_cpu(rx_desc->length); /* !EOP means multiple descriptors were used to store a single - * packet, also make sure the frame isn't just CRC only */ - if (unlikely(!(status & E1000_RXD_STAT_EOP) || (length <= 4))) { + * packet, if thats the case we need to toss it. In fact, we + * to toss every packet with the EOP bit clear and the next + * frame that _does_ have the EOP bit set, as it is by + * definition only a frame fragment + */ + if (unlikely(!(status & E1000_RXD_STAT_EOP))) + adapter->discarding = true; + + if (adapter->discarding) { /* All receives must fit into a single buffer */ E1000_DBG("%s: Receive packet consumed multiple" " buffers\n", netdev->name); /* recycle */ buffer_info->skb = skb; + if (status & E1000_RXD_STAT_EOP) + adapter->discarding = false; goto next_desc; }