Commit 40a14dea authored by Jesse Brandeburg's avatar Jesse Brandeburg Committed by David S. Miller

e1000: enhance frame fragment detection

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: default avatarJesse Brandeburg <jesse.brandeburg@intel.com>
Acked-by: default avatarNeil Horman <nhorman@tuxdriver.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent c1fa347f
...@@ -326,6 +326,8 @@ struct e1000_adapter { ...@@ -326,6 +326,8 @@ struct e1000_adapter {
/* for ioport free */ /* for ioport free */
int bars; int bars;
int need_ioport; int need_ioport;
bool discarding;
}; };
enum e1000_state_t { enum e1000_state_t {
......
...@@ -3850,13 +3850,22 @@ static bool e1000_clean_rx_irq(struct e1000_adapter *adapter, ...@@ -3850,13 +3850,22 @@ static bool e1000_clean_rx_irq(struct e1000_adapter *adapter,
length = le16_to_cpu(rx_desc->length); length = le16_to_cpu(rx_desc->length);
/* !EOP means multiple descriptors were used to store a single /* !EOP means multiple descriptors were used to store a single
* packet, also make sure the frame isn't just CRC only */ * packet, if thats the case we need to toss it. In fact, we
if (unlikely(!(status & E1000_RXD_STAT_EOP) || (length <= 4))) { * 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 */ /* All receives must fit into a single buffer */
E1000_DBG("%s: Receive packet consumed multiple" E1000_DBG("%s: Receive packet consumed multiple"
" buffers\n", netdev->name); " buffers\n", netdev->name);
/* recycle */ /* recycle */
buffer_info->skb = skb; buffer_info->skb = skb;
if (status & E1000_RXD_STAT_EOP)
adapter->discarding = false;
goto next_desc; goto next_desc;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment