control_msg_handler() passes the modem-supplied data_length field of a CTL_ID_HS2_MSG message straight to t7xx_fsm_append_event(), which memcpy()s that many bytes out of the skb. Nothing compares the field with the actual length of the message, so a modem reporting a larger data_length makes the driver read past the end of the skb and store kernel heap memory in the FSM event. The message is not even checked against the control header itself: for a message shorter than sizeof(struct ctrl_msg_header) the skb_pull() is a no-op and ctrl_msg_h->data_length is read past the skb tail. And on the consumer side, t7xx_prepare_device_rt_data() reads a full struct feature_query out of the event without looking at its length, so a short data_length makes it read past the end of the event allocation and echo the out-of-bounds bytes back to the modem in the HS3 reply. Reject the message when it is shorter than the control header or when data_length does not fit into the skb, and make t7xx_prepare_device_rt_data() bail out when the payload is shorter than struct feature_query. Fixes: da45d2566a1d ("net: wwan: t7xx: Add control port") Cc: stable@vger.kernel.org Signed-off-by: Guanglei Zhu Reviewed-by: Loic Poulain --- Changes in v2, addressing the sashiko AI review: - reject messages shorter than the control header, so ctrl_msg_h->data_length is not read past the skb tail - make t7xx_prepare_device_rt_data() length-aware: a payload shorter than struct feature_query is rejected instead of being read past the end of the event allocation and echoed back in the HS3 reply - flatten the HS2 arm nesting while at it The data_length upper-bound check was verified in a QEMU guest with a fault injector feeding the driver's control port thread a handshake message whose data_length exceeds the skb: the unpatched driver trips KASAN on a read of 8192 bytes past a 500-byte payload, and the extra bytes land in the FSM event. With this check the message is rejected, and well-formed handshakes are unaffected. drivers/net/wwan/t7xx/t7xx_modem_ops.c | 10 ++++++-- drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c | 27 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c index adb29d3..de689aa 100644 --- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c +++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c @@ -400,13 +400,18 @@ static void t7xx_prepare_host_rt_data_query(struct t7xx_sys_info *core) } static int t7xx_prepare_device_rt_data(struct t7xx_sys_info *core, struct device *dev, - void *data) + void *data, int data_length) { struct feature_query *md_feature = data; struct mtk_runtime_feature *rt_feature; unsigned int i, rt_data_len = 0; struct sk_buff *skb; + if (data_length < sizeof(*md_feature)) { + dev_err(dev, "Invalid runtime data length: %d\n", data_length); + return -EINVAL; + } + /* Parse MD runtime data query */ if (le32_to_cpu(md_feature->head_pattern) != MD_FEATURE_QUERY_ID || le32_to_cpu(md_feature->tail_pattern) != MD_FEATURE_QUERY_ID) { @@ -563,7 +568,8 @@ static void t7xx_core_hk_handler(struct t7xx_modem *md, struct t7xx_sys_info *co if (ctl->exp_flg) goto err_free_event; - ret = t7xx_prepare_device_rt_data(core_info, dev, event->data); + ret = t7xx_prepare_device_rt_data(core_info, dev, event->data, + event->length); if (ret) { dev_err(dev, "Device failure parsing runtime data: %d", ret); goto err_free_event; diff --git a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c index f869e4e..5ed07c4 100644 --- a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c +++ b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c @@ -178,22 +178,39 @@ static int control_msg_handler(struct t7xx_port *port, struct sk_buff *skb) ctrl_msg_h = (struct ctrl_msg_header *)skb->data; switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) { - case CTL_ID_HS2_MSG: - skb_pull(skb, sizeof(*ctrl_msg_h)); + case CTL_ID_HS2_MSG: { + u32 data_length; + + if (skb->len < sizeof(*ctrl_msg_h)) { + dev_err(port->dev, + "Invalid HS2 message: need %zu, have %u\n", + sizeof(*ctrl_msg_h), skb->len); + ret = -EINVAL; + dev_kfree_skb_any(skb); + break; + } - if (port_conf->rx_ch == PORT_CH_CONTROL_RX || - port_conf->rx_ch == PORT_CH_AP_CONTROL_RX) { + skb_pull(skb, sizeof(*ctrl_msg_h)); + data_length = le32_to_cpu(ctrl_msg_h->data_length); + + if (data_length > skb->len) { + dev_err(port->dev, "Invalid HS2 message length %u\n", + data_length); + ret = -EINVAL; + } else if (port_conf->rx_ch == PORT_CH_CONTROL_RX || + port_conf->rx_ch == PORT_CH_AP_CONTROL_RX) { int event = port_conf->rx_ch == PORT_CH_CONTROL_RX ? FSM_EVENT_MD_HS2 : FSM_EVENT_AP_HS2; ret = t7xx_fsm_append_event(ctl, event, skb->data, - le32_to_cpu(ctrl_msg_h->data_length)); + data_length); if (ret) dev_err(port->dev, "Failed to append Handshake 2 event"); } dev_kfree_skb_any(skb); break; + } case CTL_ID_MD_EX: case CTL_ID_MD_EX_ACK: -- 2.43.0