Re: [PATCH] scsi: mpt3sas: avoid out-of-bounds cpumask_of_node() call in _base_assign_reply_queues()

[email protected]
Newsgroups gmane.linux.scsi
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect calculation of remaining MSI-X queues causes a Division by Zero kernel panic or severe IO imbalance.
--

--- Patch [1]: [PATCH] scsi: mpt3sas: avoid out-of-bounds cpumask_of_node() call in _base_assign_reply_queues() ---
commit 2577a38cb04664cd3aa266df2234da5f8d4aff10
Author: Ivy Lopez <[email protected]>

scsi: mpt3sas: avoid out-of-bounds cpumask_of_node() call in _base_assign_reply_queues()

Fixes an out-of-bounds array read in cpumask_of_node() by checking if
dev_to_node() returns NUMA_NO_NODE. If so, it falls back to cpu_online_mask
instead of using the invalid NUMA node index.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221294

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index fed7aeffec58d..b354cbcd12741 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -3238,7 +3238,9 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
>  		 * corresponding to high iops queues.
>  		 */
>  		if (ioc->high_iops_queues) {
> -			mask = cpumask_of_node(dev_to_node(&ioc->pdev->dev));
> +			int node = (dev_to_node(&ioc->pdev->dev));
> +
> +			mask = (node == NUMA_NO_NODE) ? cpu_online_mask : cpumask_of_node(node);
>  			for (index = 0; index < ioc->high_iops_queues;
>  			    index++) {
>  				irq = pci_irq_vector(ioc->pdev, index);

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential division by zero
later in drivers/scsi/mpt3sas/mpt3sas_base.c:_base_assign_reply_queues():

fall_back:
	cpu = cpumask_first(cpu_online_mask);
	nr_msix -= (ioc->high_iops_queues - iopoll_q_count);
	index = 0;

	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
		unsigned int i, group = nr_cpus / nr_msix;

Can this result in a division by zero panic?

If iopoll_q_count == 0 and all available vectors are assigned to
high_iops_queues, nr_msix becomes 0. The list loop unconditionally computes
group = nr_cpus / nr_msix before checking if the queue is a default queue,
triggering a division by zero.

Additionally, does the fallback logic incorrectly add the poll queues instead
of subtracting them when iopoll_q_count > 0? This would shrink the CPU group
size and could cause unassigned CPUs to fall back to a single queue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.