Re: [PATCH net-next 02/11] net: ethernet: qualcomm: ppe: Fix multicast queue config table index

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: ethernet: qualcomm: out-of-bounds regmap access due to incorrect multicast_queue_cfg size
--

commit e63c30c06634bbba4d742f77aae5726bcf70fb62
Author: Luo Jie <jie.luo@oss.qualcomm.com>

net: ethernet: qualcomm: ppe: Fix multicast queue config table index

PPE queues 0-255 are unicast queues and queues from 256 onwards are
multicast queues. The multicast queue configuration table has its own
index, starting from 0 for the first multicast queue. The driver previously
used the absolute queue ID as the index, which was out of range. This commit
fixes it by converting the queue ID to the correct multicast table index.

> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> index 125b73be92b13..8a553902ffad1 100644
> --- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> @@ -1530,8 +1530,13 @@ static int ppe_config_qm(struct ppe_device *ppe_dev)
>  				if (ret)
>  					goto qm_config_fail;
>  			} else {
> +				/* The multicast queue table is indexed from 0 for
> +				 * the first multicast queue, which follows the
> +				 * unicast queues.
> +				 */
>  				reg = PPE_AC_MULTICAST_QUEUE_CFG_TBL_ADDR +
> -				      PPE_AC_MULTICAST_QUEUE_CFG_TBL_INC * queue_id;
> +				      PPE_AC_MULTICAST_QUEUE_CFG_TBL_INC *
> +				      (queue_id - PPE_AC_UNICAST_QUEUE_CFG_TBL_ENTRIES);
>  
>  				ret = regmap_bulk_read(ppe_dev->regmap, reg,
>  						       multicast_queue_cfg,
>  						       ARRAY_SIZE(multicast_queue_cfg));

[Severity: Medium]
In ppe_config_qm(), multicast_queue_cfg is declared as a 5-element array:

    u32 multicast_queue_cfg[5];

This means ARRAY_SIZE(multicast_queue_cfg) evaluates to 5 words (20 bytes).
However, the hardware table entry increment
(PPE_AC_MULTICAST_QUEUE_CFG_TBL_INC) is only 4 words (16 bytes).

Since this patch changes the index to correctly target the multicast queue
configuration table, does this regmap_bulk_read() over-read by 1 word into
the next queue's entry?

Additionally, a few lines later, regmap_bulk_write() also writes 5 words:

    ret = regmap_bulk_write(ppe_dev->regmap, reg,
                            multicast_queue_cfg,
                            ARRAY_SIZE(multicast_queue_cfg));

For the final queue in the loop, wouldn't writing 20 bytes overflow the
16-byte boundary and write 4 bytes past the end of the entire multicast
queue configuration table?

Can this out-of-bounds register access trigger a bus error or system abort
on the SoC?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-qcom_ipq5424_ppe-v1-0-16caf43e44ee@oss.qualcomm.com?part=2