Re: [PATCH v4] Kconfig: fix typos in core makefiles/code and purge remaining ghosts (v4)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] Exposing the previously inactive DVB_ULE_DEBUG code via CONFIG_DVB_ULE_DEBUG introduces race conditions on global state variables (ule_hist, ule_where, ule_dump) leading to out-of-bounds memory reads and writes.
- [Low] Enabling CONFIG_DVB_ULE_DEBUG exposes the use of the undeclared identifier `TS_SZ`, causing a compilation error.

Pre-existing issues:
- [Critical] Background workqueues are flushed before the network device is unregistered, leading to a Use-After-Free if work is rescheduled.
- [High] Lockless access to priv->in_use and direct invocation of dvb_net_stop() in dvb_net_remove_if bypasses RTNL protection.
--

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

Kconfig: fix typos in core makefiles/code and purge remaining ghosts (v4)

Following precise automated feedback from Sashiko AI, the active lcd-panel-cgram
documentation has been completely retained to prevent UAPI/Sphinx tree-wide
translation breaks. This version properly stages and includes the intended
bugfixes, such as fixing MediaTek typos, adding missing CONFIG_ prefixes, and
purging verified ghost weight.

> 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
> @@ -68,7 +68,7 @@ static inline __u32 iov_crc32( __u32 c, struct kvec *iov, unsigned int cnt )
>  
>  #define DVB_NET_MULTICAST_MAX 10
>  
> -#ifdef DVB_ULE_DEBUG
> +#ifdef CONFIG_DVB_ULE_DEBUG
>  /*
>   * The code inside DVB_ULE_DEBUG keeps a history of the
>   * last 100 TS cells processed.

[Severity: Low]
Does enabling CONFIG_DVB_ULE_DEBUG expose the use of the undeclared identifier
TS_SZ?

Building the kernel with CONFIG_DVB_ULE_DEBUG=y enabled will fail immediately.
The newly exposed debug code declares a global array ule_hist using the TS_SZ
macro:

    static unsigned char ule_hist[100*TS_SZ] = { 0 };

but TS_SZ is not defined until later in the file.

[ ... ]
> @@ -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]
Does exposing the previously inactive debug code via CONFIG_DVB_ULE_DEBUG
introduce race conditions on global state variables like ule_hist and
ule_where?

If the code is fixed to compile, dvb_net_ule_new_ts_cell() will execute without
any locks when network packets arrive. Could multiple concurrent threads
processing DVB TS cells bypass the unprotected bounds check:

    if (ule_where >= &ule_hist[100*TS_SZ])

and allow ule_where to be incremented far beyond the array bounds, leading to
arbitrary out-of-bounds memcpy() writes of incoming TS network data?

[ ... ]
> @@ -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]
Could a concurrent reset of ule_where by another thread right before the
subsequent hexdump() calls:

    hexdump(ule_where - TS_SZ - TS_SZ, TS_SZ);
    hexdump(ule_where - TS_SZ, TS_SZ);

result in a negative offset and an out-of-bounds kernel memory read in
dvb_net_ule_check_crc()?

[Severity: High]
This is a pre-existing issue, but does lockless access to priv->in_use and
direct invocation of dvb_net_stop() bypass RTNL protection?

Looking at dvb_net_remove_if() in the same file:

    priv = netdev_priv(net);
    if (priv->in_use)
        return -EBUSY;

    dvb_net_stop(net);

A concurrent interface open via "ip link set dev up" holds rtnl_lock(), calls
dvb_net_open(), increments in_use, and starts the feeds. Because of the data
race, might dvb_net_remove_if() observe a stale in_use == 0 and forcefully stop
the feeds concurrently, leading to double-free of feeds or kernel panic?

[Severity: Critical]
This is also a pre-existing issue, but are background workqueues flushed
before the network device is unregistered, leading to a Use-After-Free if work
is rescheduled?

Looking at dvb_net_remove_if() in the same file:

    flush_work(&priv->set_multicast_list_wq);
    flush_work(&priv->restart_net_feed_wq);
    pr_info("removed network interface %s\n", net->name);
    unregister_netdev(net);
    dvbnet->state[num]=0;
    dvbnet->device[num] = NULL;
    free_netdev(net);

Because unregister_netdev() hasn't been called yet, couldn't the network stack
still trigger operations like ndo_set_rx_mode that will reschedule the
workqueues immediately after the flush, causing the pending work to execute
after free_netdev() and dereference freed memory?

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