From: <subash.a.kasiviswanathan@oss.qualcomm.com>
To: "'Xiang Mei'" <xmei5@asu.edu>, <sean.tranchetti@oss.qualcomm.com>,
<netdev@vger.kernel.org>
Cc: <andrew+netdev@lunn.ch>, <davem@davemloft.net>,
<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
<linux-kernel@vger.kernel.org>, <bestswngs@gmail.com>
Subject: RE: [PATCH net v2] net: qualcomm: rmnet: validate MAP frame length before ingress parsing
Date: Wed, 1 Jul 2026 01:03:13 -0600 [thread overview]
Message-ID: <000101dd0927$b043c9b0$10cb5d10$@oss.qualcomm.com> (raw)
In-Reply-To: <20260630174110.2003121-1-xmei5@asu.edu>
> -----Original Message-----
> From: Xiang Mei <xmei5@asu.edu>
> Sent: Tuesday, June 30, 2026 11:41 AM
> To: subash.a.kasiviswanathan@oss.qualcomm.com;
> sean.tranchetti@oss.qualcomm.com; netdev@vger.kernel.org
> Cc: andrew+netdev@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; linux-
> kernel@vger.kernel.org; bestswngs@gmail.com; Xiang Mei <xmei5@asu.edu>
> Subject: [PATCH net v2] 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>
> ---
> v2: Validate on the no-aggregation path by reusing the deaggregation
> length checks (factored into rmnet_map_validate_packet_len()) instead
> of adding separate pskb_may_pull() guards in
> __rmnet_map_ingress_handler().
>
> .../ethernet/qualcomm/rmnet/rmnet_handlers.c | 5 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_map.h | 1 +
> .../ethernet/qualcomm/rmnet/rmnet_map_data.c | 72 ++++++++++---------
> 3 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 9f3479500f85..d055a2628d8c 100644
> --- 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 b70284095568..60ca8b780c88 100644
> --- 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 8b4640c5d61e..305ae15ae8f3 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -333,54 +333,62 @@ struct rmnet_map_header
> *rmnet_map_add_map_header(struct sk_buff *skb,
> 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)
> --
> 2.43.0
Reviewed-by: Subash Abhinov Kasiviswanathan
<subash.a.kasiviswanathan@oss.qualcomm.com>
prev parent reply other threads:[~2026-07-01 7:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-30 17:41 [PATCH net v2] net: qualcomm: rmnet: validate MAP frame length before ingress parsing Xiang Mei
2026-06-30 17:43 ` Xiang Mei
2026-07-01 7:03 ` subash.a.kasiviswanathan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='000101dd0927$b043c9b0$10cb5d10$@oss.qualcomm.com' \
--to=subash.a.kasiviswanathan@oss.qualcomm.com \
--cc=andrew+netdev@lunn.ch \
--cc=bestswngs@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sean.tranchetti@oss.qualcomm.com \
--cc=xmei5@asu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox