net: qualcomm: rmnet: validate MAP frame length before ingress parsing

When ingress deaggregation is disabled, rmnet_map_ingress_handler() passes
the skb straight to __rmnet_map_ingress_handler(), skipping the length
validation that rmnet_map_deaggregate() performs on the aggregated path.
The parser then dereferences the MAP header and csum header/trailer based on
the on-wire pkt_len without checking skb->len, so a short frame is read out
of bounds:

  BUG: KASAN: slab-out-of-bounds in rmnet_map_checksum_downlink_packet
  Read of size 1 at addr ffff88801118ed00 by task exploit/147
  Call Trace:
   ...
   rmnet_map_checksum_downlink_packet (drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:413)
   __rmnet_map_ingress_handler (drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c:96)
   rmnet_rx_handler (drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c:129)
   __netif_receive_skb_core.constprop.0 (net/core/dev.c:6089)
   netif_receive_skb (net/core/dev.c:6460)
   tun_get_user (drivers/net/tun.c:1955)
   tun_chr_write_iter (drivers/net/tun.c:2001)
   vfs_write (fs/read_write.c:688)
   ksys_write (fs/read_write.c:740)
   do_syscall_64 (arch/x86/entry/syscall_64.c:94)
   ...

Factor that validation out of rmnet_map_deaggregate() into
rmnet_map_validate_packet_len() and run it on the no-aggregation path too.
The MAP header is bounds-checked first, since this path can receive a frame
shorter than the header.

Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Suggested-by: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Link: https://patch.msgid.link/20260630174110.2003121-1-xmei5@asu.edu
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 9f3479500f85a1d52d56331ef5d1966ccb734ad2..d055a2628d8c9d0b6e7e85eb98f6fa9b5a1bd531 100644 (file)
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -126,7 +126,10 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
 
                consume_skb(skb);
        } else {
-               __rmnet_map_ingress_handler(skb, port);
+               if (rmnet_map_validate_packet_len(skb, port))
+                       __rmnet_map_ingress_handler(skb, port);
+               else
+                       kfree_skb(skb);
        }
 }
 

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index b70284095568ce32b0cd1b8f91e6fa4334b32817..60ca8b780c88ad7d9a6334370c0ff73a83e02d2d 100644 (file)
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -59,5 +59,6 @@ void rmnet_map_tx_aggregate_init(struct rmnet_port *port);
 void rmnet_map_tx_aggregate_exit(struct rmnet_port *port);
 void rmnet_map_update_ul_agg_config(struct rmnet_port *port, u32 size,
                                    u32 count, u32 time);
+u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port);
 
 #endif /* _RMNET_MAP_H_ */

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 8b4640c5d61ebe854ae54ec7decf649d50097f1a..305ae15ae8f393ecbe0749cdca3f55f2a5159edf 100644 (file)
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -333,54 +333,62 @@ done:
        return map_header;
 }
 
-/* Deaggregates a single packet
- * A whole new buffer is allocated for each portion of an aggregated frame.
- * Caller should keep calling deaggregate() on the source skb until 0 is
- * returned, indicating that there are no more packets to deaggregate. Caller
- * is responsible for freeing the original skb.
- */
-struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
-                                     struct rmnet_port *port)
+u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
 {
        struct rmnet_map_v5_csum_header *next_hdr = NULL;
        struct rmnet_map_header *maph;
        void *data = skb->data;
-       struct sk_buff *skbn;
-       u8 nexthdr_type;
        u32 packet_len;
 
-       if (skb->len == 0)
-               return NULL;
+       if (skb->len < sizeof(*maph))
+               return 0;
 
        maph = (struct rmnet_map_header *)skb->data;
+
+       /* Some hardware can send us empty frames. Catch them */
+       if (!maph->pkt_len)
+               return 0;
+
        packet_len = ntohs(maph->pkt_len) + sizeof(*maph);
 
        if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
                packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
-       } else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
-               if (!(maph->flags & MAP_CMD_FLAG)) {
-                       packet_len += sizeof(*next_hdr);
-                       if (maph->flags & MAP_NEXT_HEADER_FLAG)
-                               next_hdr = data + sizeof(*maph);
-                       else
-                               /* Mapv5 data pkt without csum hdr is invalid */
-                               return NULL;
-               }
+       } else if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
+                  !(maph->flags & MAP_CMD_FLAG)) {
+               /* Mapv5 data pkt without csum hdr is invalid */
+               if (!(maph->flags & MAP_NEXT_HEADER_FLAG))
+                       return 0;
+
+               packet_len += sizeof(*next_hdr);
+               next_hdr = data + sizeof(*maph);
        }
 
-       if (((int)skb->len - (int)packet_len) < 0)
-               return NULL;
+       if (skb->len < packet_len)
+               return 0;
 
-       /* Some hardware can send us empty frames. Catch them */
-       if (!maph->pkt_len)
-               return NULL;
+       if (next_hdr &&
+           u8_get_bits(next_hdr->header_info, MAPV5_HDRINFO_HDR_TYPE_FMASK) !=
+           RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
+               return 0;
 
-       if (next_hdr) {
-               nexthdr_type = u8_get_bits(next_hdr->header_info,
-                                          MAPV5_HDRINFO_HDR_TYPE_FMASK);
-               if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
-                       return NULL;
-       }
+       return packet_len;
+}
+
+/* Deaggregates a single packet
+ * A whole new buffer is allocated for each portion of an aggregated frame.
+ * Caller should keep calling deaggregate() on the source skb until 0 is
+ * returned, indicating that there are no more packets to deaggregate. Caller
+ * is responsible for freeing the original skb.
+ */
+struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
+                                     struct rmnet_port *port)
+{
+       struct sk_buff *skbn;
+       u32 packet_len;
+
+       packet_len = rmnet_map_validate_packet_len(skb, port);
+       if (!packet_len)
+               return NULL;
 
        skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
        if (!skbn)