net: aquantia: memory corruption on jumbo frames

This patch fixes skb_shared area, which will be corrupted
upon reception of 4K jumbo packets.

Originally build_skb usage purpose was to reuse page for skb to eliminate
needs of extra fragments. But that logic does not take into account that
skb_shared_info should be reserved at the end of skb data area.

In case packet data consumes all the page (4K), skb_shinfo location
overflows the page. As a consequence, __build_skb zeroed shinfo data above
the allocated page, corrupting next page.

The issue is rarely seen in real life because jumbo are normally larger
than 4K and that causes another code path to trigger.
But it 100% reproducible with simple scapy packet, like:

    sendp(IP(dst="192.168.100.3") / TCP(dport=443) \
          / Raw(RandString(size=(4096-40))), iface="enp1s0")

Fixes: 018423e90bee ("net: ethernet: aquantia: Add ring support code")
Reported-by: Friedemann Gerold <f.gerold@b-c-s.de>
Reported-by: Michael Rauch <michael@rauch.be>
Signed-off-by: Friedemann Gerold <f.gerold@b-c-s.de>
Tested-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 32111272d7bf9bddd19bf8a60de4b70ffef2fedb..3db91446cc67717b548333e4bd3b95db8281c152 100644 (file)
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -225,9 +225,10 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
                }
 
                /* for single fragment packets use build_skb() */
-               if (buff->is_eop) {
+               if (buff->is_eop &&
+                   buff->len <= AQ_CFG_RX_FRAME_MAX - AQ_SKB_ALIGN) {
                        skb = build_skb(page_address(buff->page),
-                                       buff->len + AQ_SKB_ALIGN);
+                                       AQ_CFG_RX_FRAME_MAX);
                        if (unlikely(!skb)) {
                                err = -ENOMEM;
                                goto err_exit;
@@ -247,18 +248,21 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
                                        buff->len - ETH_HLEN,
                                        SKB_TRUESIZE(buff->len - ETH_HLEN));
 
-                       for (i = 1U, next_ = buff->next,
-                            buff_ = &self->buff_ring[next_]; true;
-                            next_ = buff_->next,
-                            buff_ = &self->buff_ring[next_], ++i) {
-                               skb_add_rx_frag(skb, i, buff_->page, 0,
-                                               buff_->len,
-                                               SKB_TRUESIZE(buff->len -
-                                               ETH_HLEN));
-                               buff_->is_cleaned = 1;
-
-                               if (buff_->is_eop)
-                                       break;
+                       if (!buff->is_eop) {
+                               for (i = 1U, next_ = buff->next,
+                                    buff_ = &self->buff_ring[next_];
+                                    true; next_ = buff_->next,
+                                    buff_ = &self->buff_ring[next_], ++i) {
+                                       skb_add_rx_frag(skb, i,
+                                                       buff_->page, 0,
+                                                       buff_->len,
+                                                       SKB_TRUESIZE(buff->len -
+                                                       ETH_HLEN));
+                                       buff_->is_cleaned = 1;
+
+                                       if (buff_->is_eop)
+                                               break;
+                               }
                        }
                }