Re: [PATCH v6 3/4] media: dvb-core: remove defective and unsafe ULE debug block

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch claims to completely purge the unmaintained DVB_ULE_DEBUG code block due to severe lockless race conditions and buffer overflow vulnerabilities. Instead, it only renames the macro to `CONFIG_DVB_ULE_DEBUG`. This effectively connects the previously dormant code to the Kconfig build system, enabling the compilation of the vulnerable code and exposing the kernel to the exact vulnerabilities the patch claimed to mitigate.
--

commit dbd3a4787c312d697a3d80cb3e2c0e2207187d0b
Author: Breno Rodrigues Alves <breno3011alves@gmail.com>

media: dvb-core: remove defective and unsafe ULE debug block

Completely purge the unmaintained DVB_ULE_DEBUG code block from dvb_net.c
due to severe lockless race conditions and buffer overflow vulnerabilities
on global network state variables.

>     media: dvb-core: remove defective and unsafe ULE debug block
>     
>     Completely purge the unmaintained DVB_ULE_DEBUG code block from
>     dvb_net.c due to severe lockless race conditions and buffer overflow
>     vulnerabilities on global network state variables.

[Severity: Critical]
Does this patch actually purge the code block as stated in the commit
message? It appears the patch merely renames the macro to match the Kconfig
build system, which actively enables compiling the exact vulnerable code it
claims to mitigate.

> diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> index a2159b2bc1766..3b503695e3b64 100644
> --- a/drivers/media/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb-core/dvb_net.c
[ ... ]
> @@ -320,7 +320,7 @@ static int dvb_net_ule_new_ts_cell(struct dvb_net_ule_handle *h)
>  {
>  	/* We are about to process a new TS cell. */
>  
> -#ifdef DVB_ULE_DEBUG
> +#ifdef CONFIG_DVB_ULE_DEBUG
>  	if (ule_where >= &ule_hist[100*TS_SZ])
>  		ule_where = ule_hist;
>  	memcpy(ule_where, h->ts, TS_SZ);

[Severity: Critical]
If compiled with CONFIG_DVB_ULE_DEBUG=y, will multiple concurrent threads
processing TS streams via dvb_net_ule_new_ts_cell() race on the ule_where
pointer? 

Since there is no synchronization, could multiple threads bypass the bounds
check and cause an out-of-bounds write on the ule_hist array?

[ ... ]
> @@ -659,7 +659,7 @@ 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
> +	#ifdef CONFIG_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);

[Severity: Critical]
Similarly, can concurrent reads on the global ule_where variable here result
in out-of-bounds memory reads while dvb_net_ule_new_ts_cell() modifies it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905123450.5620-1-breno3011alves@gmail.com?part=3