Re: [PATCH 2/3] net: wwan: mhi_wwan_mbim: check skb_copy_bits() return value

From
Loic Poulain <loic.poulain@oss.qualcomm.com>
Date
2026-09-10 08:09:29
Also in
lkml, stable
Reply to
Guanglei Zhu, 2026-09-10
On Thu, Sep 10, 2026 at 5:34 AM Guanglei Zhu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
mhi_mbim_rx() ignores the return value of skb_copy_bits() when it
copies each datagram out of the NTB.  The datagram offset and length
come from the DPE, which is only checked to lie within the NTB
itself, so a modem can point a datagram outside the received skb.
The copy then fails and the freshly allocated skbn is passed to
netif_rx() with its uninitialized contents still in place, leaking
kernel heap memory into the network stack.

Free the skb and account an error when the copy fails.

Fixes: aa730a9905b7 ("net: wwan: Add MHI MBIM network driver")
Cc: <redacted>
Signed-off-by: Guanglei Zhu <redacted>
---

Verified in a QEMU guest with a fault injector pointing a DPE
outside the received NTB: the copy fails, and the unpatched driver
hands the uninitialized skbn to the network stack (observed as
"unknown protocol" on bytes that were never written).  With this
check the failed datagram is dropped and counted as an rx error.
 drivers/net/wwan/mhi_wwan_mbim.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wwan/mhi_wwan_mbim.c b/drivers/net/wwan/mhi_wwan_mbim.c
index ef158edeb..71e700008 100644
--- a/drivers/net/wwan/mhi_wwan_mbim.c
+++ b/drivers/net/wwan/mhi_wwan_mbim.c
@@ -328,7 +328,13 @@ static void mhi_mbim_rx(struct mhi_mbim_context *mbim, struct sk_buff *skb)
                                continue;

                        skb_put(skbn, dgram_len);
-                       skb_copy_bits(skb, dgram_offset, skbn->data, dgram_len);
+                       if (skb_copy_bits(skb, dgram_offset, skbn->data, dgram_len)) {
+                               dev_kfree_skb_any(skbn);
+                               u64_stats_update_begin(&link->rx_syncp);
+                               u64_stats_inc(&link->rx_errors);
+                               u64_stats_update_end(&link->rx_syncp);
+                               continue;
We perform the same error handling in the switch cases below, so it
would be better to factor it out into a common helper or a shared
error path.
+                       }

                        switch (skbn->data[0] & 0xf0) {
                        case 0x40:
--
2.43.0
Thread (9 messages) 9 messages, 3 authors, last reply 2026-09-17
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help