Re: [PATCH v2] net: wwan: t7xx: validate port_count against message length in t7xx_port_enum_msg_handler

Willy Tarreau
2026-04-14T16:23:32Z

Hello,

On Tue, Apr 14, 2026 at 11:31:56AM -0400, Pavitra Jha wrote:
> t7xx_port_enum_msg_handler() uses the modem-supplied port_count field as
> a loop bound over port_msg->data[] without checking that the message buffer
> contains sufficient data. A modem sending port_count=65535 in a 12-byte
> buffer triggers a slab-out-of-bounds read of up to 262140 bytes.
>
> Add a struct_size() check after extracting port_count and before the loop.
> Pass msg_len to t7xx_port_enum_msg_handler() and use it to validate
> the message size before accessing port_msg->data[].
> Pass msg_len from both call sites: skb->len at the DPMAIF path after
> skb_pull(), and the captured rt_feature->data_len at the handshake path.
>
> Fixes: 39d439047f1d ("net: wwan: t7xx: Add control DMA interface")
> Cc: stable@vger.kernel.org
> Reported-by: Pavitra Jha <jhapavitra98@gmail.com>
> Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com>

Please note that you don't need the Reported-by tag when it's the same
as the Signed-off-by one.

Also, I'm noticing a few empty-line removals out of context below:

> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> index 7968e208d..d0559fe16 100644
> --- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> +++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> @@ -453,25 +453,25 @@ static int t7xx_parse_host_rt_data(struct t7xx_fsm_ctl *ctl, struct t7xx_sys_inf
> {
> enum mtk_feature_support_type ft_spt_st, ft_spt_cfg;
> struct mtk_runtime_feature *rt_feature;
> + size_t feat_data_len;
> int i, offset;
>
> offset = sizeof(struct feature_query);
> for (i = 0; i < FEATURE_COUNT && offset < data_length; i++) {
> rt_feature = data + offset;
> - offset += sizeof(*rt_feature) + le32_to_cpu(rt_feature->data_len);
> -
> + feat_data_len = le32_to_cpu(rt_feature->data_len);
> + offset += sizeof(*rt_feature) + feat_data_len;
> ft_spt_cfg = FIELD_GET(FEATURE_MSK, core->feature_set[i]);
> if (ft_spt_cfg != MTK_FEATURE_MUST_BE_SUPPORTED)
> continue;
> -

here

> ft_spt_st = FIELD_GET(FEATURE_MSK, rt_feature->support_info);
> if (ft_spt_st != MTK_FEATURE_MUST_BE_SUPPORTED)
> return -EINVAL;
> -

Here, the original author probably left the line to highlight the return
statement.

> - if (i == RT_ID_MD_PORT_ENUM || i == RT_ID_AP_PORT_ENUM)
> - t7xx_port_enum_msg_handler(ctl->md, rt_feature->data);
> + if (i == RT_ID_MD_PORT_ENUM || i == RT_ID_AP_PORT_ENUM) {
> + t7xx_port_enum_msg_handler(ctl->md, rt_feature->data,
> + feat_data_len);
> + }
> }
> -

Here, why?

> return 0;
> }
>
> diff --git a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c
> index ae632ef96..d984a688d 100644
> --- a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c
> +++ b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c
> @@ -154,7 +161,6 @@ int t7xx_port_enum_msg_handler(struct t7xx_modem *md, void *msg)
>
> return 0;
> }
> -

This one as well.

> static int control_msg_handler(struct t7xx_port *port, struct sk_buff *skb)
> {
> const struct t7xx_port_conf *port_conf = port->port_conf;

Better leave them untouched, it will keep the code as readable as it
previously was and reduce the overall review effort.

thanks,
willy