Re: [PATCH v8 3/3] media: dvb-core: extirpate defective and unsafe ULE debug block
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-05 13:06:35
Also in:
linux-clk, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The commit message explicitly claims to 'clean up unused function parameters', but the `iov` parameter in `dvb_net_ule_check_crc()` is left intact and completely unused.
Pre-existing issues:
- [Critical] Insufficient length checks during ULE decapsulation allow out-of-bounds reads from kernel memory, leading to an information leak and potential kernel crash.
--
commit bc7f64f0594cb54302cffa558b837ac23e377b2e
Author: Breno Rodrigues Alves [off-list ref]
media: dvb-core: extirpate defective and unsafe ULE debug block
This patch removes the unmaintained DVB_ULE_DEBUG code block from the dvb-core
network code, cleaning up its Kconfig definition and logging logic. It also
claims to clean up unused function parameters.
Completely purge the unmaintained DVB_ULE_DEBUG code block from
dvb_net.c and its Kconfig definition to eliminate severe lockless race
conditions and clean up unused function parameters.
[Severity: Low]
The commit message claims to "clean up unused function parameters", but
is the iov parameter still present and completely unused in
dvb_net_ule_check_crc() after this removal?
It appears the caller in dvb_net_ule() still initializes and passes iov:
struct kvec iov[3] = {
{ &ulen, sizeof ulen },
{ &utype, sizeof utype },
{ h.priv->ule_skb->data,
h.priv->ule_skb->len - 4 }
};
...
dvb_net_ule_check_crc(&h, iov, ule_crc, expected_crc);
Should iov be removed from the function signature and the caller as well?
quoted hunk ↗ jump to hunk
diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c index a2159b2bc1766..ae0073b949a50 100644 --- a/drivers/media/dvb-core/dvb_net.c +++ b/drivers/media/dvb-core/dvb_net.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -659,23 +636,6 @@ static void dvb_net_ule_check_crc(struct dvb_net_ule_handle *h, h->ts_remain > 2 ? *(unsigned short *)h->from_where : 0); - #ifdef DVB_ULE_DEBUG - hexdump(iov[0].iov_base, iov[0].iov_len); - hexdump(iov[1].iov_base, iov[1].iov_len); - hexdump(iov[2].iov_base, iov[2].iov_len); - - if (ule_where == ule_hist) { - hexdump(&ule_hist[98*TS_SZ], TS_SZ); - hexdump(&ule_hist[99*TS_SZ], TS_SZ); - } else if (ule_where == &ule_hist[TS_SZ]) { - hexdump(&ule_hist[99*TS_SZ], TS_SZ); - hexdump(ule_hist, TS_SZ); - } else { - hexdump(ule_where - TS_SZ - TS_SZ, TS_SZ); - hexdump(ule_where - TS_SZ, TS_SZ); - } - ule_dump = 1; - #endif h->dev->stats.rx_errors++; h->dev->stats.rx_crc_errors++;
[Severity: Critical] This is a pre-existing issue, but does this code properly check boundaries before pulling from the SKB? Further down in dvb_net_ule_check_crc(), we blindly copy and pull ETH_ALEN: skb_copy_from_linear_data(h->priv->ule_skb, dest_addr, ETH_ALEN); skb_pull(h->priv->ule_skb, ETH_ALEN); If skb->len is very small (e.g., ule_sndu_len < 10), skb_pull() fails silently, leaving the data pointer unadvanced. Later, handle_one_ule_extension() blindly advances the header pointer by hlen << 1 without bounds checking: ext_len = hlen << 1; if (ule_optional_ext_handlers[htype]) (void)ule_optional_ext_handlers[htype]( p ); p->ule_next_hdr += ext_len; p->ule_sndu_type = ntohs( *(__be16 *)(p->ule_next_hdr-2) ); Can this sequence lead to an out-of-bounds slab read if an attacker broadcasts a malformed DVB transport stream cell with extension headers indicating a large length? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260905125706.6550-1-breno3011alves@gmail.com?part=3