* [PATCH 0/2] Drop SCSI core dev->dma_mask check
@ 2026-06-29 8:53 John Garry
2026-06-29 8:53 ` [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability John Garry
2026-06-29 8:53 ` [PATCH 2/2] scsi: core: Drop dev->dma_mask check in evaluating max_sectors John Garry
0 siblings, 2 replies; 7+ messages in thread
From: John Garry @ 2026-06-29 8:53 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, m.szyprowski, robin.murphy, hch
Cc: linux-scsi, iommu, ionut.nechita, John Garry
In commit be8fcd4a8217 ("scsi: sas: Skip opt_sectors when DMA reports no
real optimization hint"), the check for dev->dma_mask prior to calling
dma_opt_mapping_size() was dropped.
However, it is not safe to do so, as dma_opt_mapping_size() ->
dma_max_mapping_size() may try to dereference dev->dma_mask.
We just don't hit such a path as no SCSI HBA driver which has
dev->dma_mask unset would use the SAS transport.
Fix that issue by adding a dev->dma_mask check in
dma_max_mapping_size().
The dev->dma_mask check in scsi_add_host_with_dma() can then be also
dropped.
John Garry (2):
dma-mapping: make dma_max_mapping_size() return 0 for no DMA
capability
scsi: core: Drop dev->dma_mask check in evaluating max_sectors
drivers/scsi/hosts.c | 6 ++----
kernel/dma/mapping.c | 3 +++
2 files changed, 5 insertions(+), 4 deletions(-)
--
2.43.7
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability 2026-06-29 8:53 [PATCH 0/2] Drop SCSI core dev->dma_mask check John Garry @ 2026-06-29 8:53 ` John Garry 2026-06-29 9:10 ` sashiko-bot 2026-06-29 10:11 ` Robin Murphy 2026-06-29 8:53 ` [PATCH 2/2] scsi: core: Drop dev->dma_mask check in evaluating max_sectors John Garry 1 sibling, 2 replies; 7+ messages in thread From: John Garry @ 2026-06-29 8:53 UTC (permalink / raw) To: James.Bottomley, martin.petersen, m.szyprowski, robin.murphy, hch Cc: linux-scsi, iommu, ionut.nechita, John Garry For when a device is not DMA capable, the max mapping size would be 0, so make dma_max_mapping_size() reflect that. Signed-off-by: John Garry <john.g.garry@oracle.com> --- kernel/dma/mapping.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 4fe04669e5e66..7e576e5c6b8be 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -979,6 +979,9 @@ size_t dma_max_mapping_size(struct device *dev) const struct dma_map_ops *ops = get_dma_ops(dev); size_t size = SIZE_MAX; + if (!dev->dma_mask) + return 0; + if (dma_map_direct(dev, ops)) size = dma_direct_max_mapping_size(dev); else if (use_dma_iommu(dev)) -- 2.43.7 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability 2026-06-29 8:53 ` [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability John Garry @ 2026-06-29 9:10 ` sashiko-bot 2026-06-29 10:11 ` Robin Murphy 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-06-29 9:10 UTC (permalink / raw) To: John Garry; +Cc: linux-scsi Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] dma_max_mapping_size() returning 0 causes division by zero in amdxdna_cbuf_map() and invalid zero-sized block layer limits in MMC drivers. -- commit b19cb5dc0c3fd9b25263f65c9d042ee616692807 Author: John Garry <john.g.garry@oracle.com> dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability This commit updates dma_max_mapping_size() to return 0 when a device is not DMA capable, reflecting that the maximum mapping size for such devices is 0. > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > index 4fe04669e5e66..7e576e5c6b8be 100644 > --- a/kernel/dma/mapping.c > +++ b/kernel/dma/mapping.c > @@ -979,6 +979,9 @@ size_t dma_max_mapping_size(struct device *dev) > const struct dma_map_ops *ops = get_dma_ops(dev); > size_t size = SIZE_MAX; > > + if (!dev->dma_mask) > + return 0; [Severity: High] Could this new return value of 0 cause a division by zero in callers that expect a strictly positive maximum mapping size? For instance, in drivers/accel/amdxdna/amdxdna_cbuf.c:amdxdna_cbuf_map(), the returned size is used directly as a divisor: max_seg = min_t(size_t, UINT_MAX, dma_max_mapping_size(dev)); n_entries = (cbuf->node.size + max_seg - 1) / max_seg; If a device lacking a dma_mask maps the attachment, max_seg becomes 0, which would result in an immediate kernel panic. Additionally, could this cause invalid zero-sized block layer queue limits in MMC drivers? In drivers/mmc/host/bcm2835.c:bcm2835_add_host(), the return value clamps max_req_size: mmc->max_req_size = min_t(size_t, 524288, dma_max_mapping_size(dev)); If the device lacks DMA support, max_req_size becomes 0, which propagates to the block layer and can trigger panics when processing requests. > + > if (dma_map_direct(dev, ops)) > size = dma_direct_max_mapping_size(dev); > else if (use_dma_iommu(dev)) -- Sashiko AI review · https://sashiko.dev/#/patchset/20260629085310.2298552-1-john.g.garry@oracle.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability 2026-06-29 8:53 ` [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability John Garry 2026-06-29 9:10 ` sashiko-bot @ 2026-06-29 10:11 ` Robin Murphy 2026-06-29 10:46 ` John Garry 1 sibling, 1 reply; 7+ messages in thread From: Robin Murphy @ 2026-06-29 10:11 UTC (permalink / raw) To: John Garry, James.Bottomley, martin.petersen, m.szyprowski, hch Cc: linux-scsi, iommu, ionut.nechita On 29/06/2026 9:53 am, John Garry wrote: > For when a device is not DMA capable, the max mapping size would be 0, so > make dma_max_mapping_size() reflect that. Seems logical. Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: John Garry <john.g.garry@oracle.com> > --- > kernel/dma/mapping.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > index 4fe04669e5e66..7e576e5c6b8be 100644 > --- a/kernel/dma/mapping.c > +++ b/kernel/dma/mapping.c > @@ -979,6 +979,9 @@ size_t dma_max_mapping_size(struct device *dev) > const struct dma_map_ops *ops = get_dma_ops(dev); > size_t size = SIZE_MAX; > > + if (!dev->dma_mask) > + return 0; > + > if (dma_map_direct(dev, ops)) > size = dma_direct_max_mapping_size(dev); > else if (use_dma_iommu(dev)) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability 2026-06-29 10:11 ` Robin Murphy @ 2026-06-29 10:46 ` John Garry 2026-06-29 11:09 ` Robin Murphy 0 siblings, 1 reply; 7+ messages in thread From: John Garry @ 2026-06-29 10:46 UTC (permalink / raw) To: Robin Murphy, James.Bottomley, martin.petersen, m.szyprowski, hch Cc: linux-scsi, iommu, ionut.nechita On 29/06/2026 11:11, Robin Murphy wrote: > On 29/06/2026 9:53 am, John Garry wrote: >> For when a device is not DMA capable, the max mapping size would be 0, so >> make dma_max_mapping_size() reflect that. > > Seems logical. > > Reviewed-by: Robin Murphy <robin.murphy@arm.com> Thanks, but sashiko has been reviewing this and has a few things to say. I don't know how it chooses recipients. On 29/06/2026 10:10, sashiko-bot@kernel.org wrote: >> >> + if (!dev->dma_mask) >> + return 0; > [Severity: High] > Could this new return value of 0 cause a division by zero in callers that > expect a strictly positive maximum mapping size? > > For instance, in drivers/accel/amdxdna/amdxdna_cbuf.c:amdxdna_cbuf_map(), > the returned size is used directly as a divisor: > > max_seg = min_t(size_t, UINT_MAX, dma_max_mapping_size(dev)); > n_entries = (cbuf->node.size + max_seg - 1) / max_seg; Later in amdxdna_cbuf_map() we call dma_map_resource() (-> dma_map_phys()) and this would fail and WARN for !dev->dma_mask. Indeed amdxdna_cbuf_map() is used as a callback for .map_dma_buf, so highly unrealistic to have !dev->dma_mask ever. > > If a device lacking a dma_mask maps the attachment, max_seg becomes 0, > which would result in an immediate kernel panic. > > Additionally, could this cause invalid zero-sized block layer queue limits > in MMC drivers? In drivers/mmc/host/bcm2835.c:bcm2835_add_host(), the > return value clamps max_req_size: > > mmc->max_req_size = min_t(size_t, 524288, dma_max_mapping_size(dev)); bcm2835.c is a platform device driver, and platform devices have their dev->dma_mask set in setup_pdev_dma_masks() Indeed, that driver does have a non-DMA mode of operation, but that looks to be selected independent of whether dev->dma_mask is set. > > If the device lacks DMA support, max_req_size becomes 0, which propagates > to the block layer and can trigger panics when processing requests. All other users of dma_max_mapping_size() are drivers for real/virtio HW, so should be no issues. > >> Signed-off-by: John Garry <john.g.garry@oracle.com> >> --- >> kernel/dma/mapping.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c >> index 4fe04669e5e66..7e576e5c6b8be 100644 >> --- a/kernel/dma/mapping.c >> +++ b/kernel/dma/mapping.c >> @@ -979,6 +979,9 @@ size_t dma_max_mapping_size(struct device *dev) >> const struct dma_map_ops *ops = get_dma_ops(dev); >> size_t size = SIZE_MAX; >> + if (!dev->dma_mask) >> + return 0; >> + >> if (dma_map_direct(dev, ops)) >> size = dma_direct_max_mapping_size(dev); >> else if (use_dma_iommu(dev)) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability 2026-06-29 10:46 ` John Garry @ 2026-06-29 11:09 ` Robin Murphy 0 siblings, 0 replies; 7+ messages in thread From: Robin Murphy @ 2026-06-29 11:09 UTC (permalink / raw) To: John Garry, James.Bottomley, martin.petersen, m.szyprowski, hch Cc: linux-scsi, iommu, ionut.nechita On 29/06/2026 11:46 am, John Garry wrote: > On 29/06/2026 11:11, Robin Murphy wrote: >> On 29/06/2026 9:53 am, John Garry wrote: >>> For when a device is not DMA capable, the max mapping size would be >>> 0, so >>> make dma_max_mapping_size() reflect that. >> >> Seems logical. >> >> Reviewed-by: Robin Murphy <robin.murphy@arm.com> > > Thanks, but sashiko has been reviewing this and has a few things to say. > > I don't know how it chooses recipients. > > On 29/06/2026 10:10, sashiko-bot@kernel.org wrote: > >> > >> + if (!dev->dma_mask) > >> + return 0; > > [Severity: High] > > Could this new return value of 0 cause a division by zero in callers > that > > expect a strictly positive maximum mapping size? > > > > For instance, in drivers/accel/amdxdna/ > amdxdna_cbuf.c:amdxdna_cbuf_map(), > > the returned size is used directly as a divisor: > > > > max_seg = min_t(size_t, UINT_MAX, dma_max_mapping_size(dev)); > > n_entries = (cbuf->node.size + max_seg - 1) / max_seg; > > Later in amdxdna_cbuf_map() we call dma_map_resource() (-> > dma_map_phys()) and this would fail and WARN for !dev->dma_mask. Indeed > amdxdna_cbuf_map() is used as a callback for .map_dma_buf, so highly > unrealistic to have !dev->dma_mask ever. Moreover, the driver would have failed to probe in the first place if its dma_set_mask_and_coherent() call had failed. > > > > If a device lacking a dma_mask maps the attachment, max_seg becomes 0, > > which would result in an immediate kernel panic. > > > > Additionally, could this cause invalid zero-sized block layer queue > limits > > in MMC drivers? In drivers/mmc/host/bcm2835.c:bcm2835_add_host(), the > > return value clamps max_req_size: > > > > mmc->max_req_size = min_t(size_t, 524288, > dma_max_mapping_size(dev)); > > bcm2835.c is a platform device driver, and platform devices have their > dev->dma_mask set in setup_pdev_dma_masks() > > Indeed, that driver does have a non-DMA mode of operation, but that > looks to be selected independent of whether dev->dma_mask is set. That one seems pretty bogus already, given that DMA mode is apparently dependent on an external DMA channel, so "dev" is the wrong device to check (should be dmaengine_get_dma_device()), while conversely dma_max_mapping_size(anything) is a questionably meaningless number for PIO mode... :/ Cheers, Robin. > > > > > If the device lacks DMA support, max_req_size becomes 0, which > propagates > > to the block layer and can trigger panics when processing requests. > > All other users of dma_max_mapping_size() are drivers for real/virtio > HW, so should be no issues. > >> >>> Signed-off-by: John Garry <john.g.garry@oracle.com> >>> --- >>> kernel/dma/mapping.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c >>> index 4fe04669e5e66..7e576e5c6b8be 100644 >>> --- a/kernel/dma/mapping.c >>> +++ b/kernel/dma/mapping.c >>> @@ -979,6 +979,9 @@ size_t dma_max_mapping_size(struct device *dev) >>> const struct dma_map_ops *ops = get_dma_ops(dev); >>> size_t size = SIZE_MAX; >>> + if (!dev->dma_mask) >>> + return 0; >>> + >>> if (dma_map_direct(dev, ops)) >>> size = dma_direct_max_mapping_size(dev); >>> else if (use_dma_iommu(dev)) >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] scsi: core: Drop dev->dma_mask check in evaluating max_sectors 2026-06-29 8:53 [PATCH 0/2] Drop SCSI core dev->dma_mask check John Garry 2026-06-29 8:53 ` [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability John Garry @ 2026-06-29 8:53 ` John Garry 1 sibling, 0 replies; 7+ messages in thread From: John Garry @ 2026-06-29 8:53 UTC (permalink / raw) To: James.Bottomley, martin.petersen, m.szyprowski, robin.murphy, hch Cc: linux-scsi, iommu, ionut.nechita, John Garry When evaluating shost->max_sectors, we currently check dma_dev->dma_mask is non-NULL, as dma_max_mapping_size(dma_dev) could previously not handle unset dma_dev->dma_mask - this is no longer the case. Signed-off-by: John Garry <john.g.garry@oracle.com> --- drivers/scsi/hosts.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index e047747d4ecf8..d512080268afe 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -252,10 +252,8 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, shost->dma_dev = dma_dev; - if (dma_dev->dma_mask) { - shost->max_sectors = min_t(unsigned int, shost->max_sectors, - dma_max_mapping_size(dma_dev) >> SECTOR_SHIFT); - } + shost->max_sectors = min_not_zero(shost->max_sectors, + dma_max_mapping_size(dma_dev) >> SECTOR_SHIFT); error = scsi_mq_setup_tags(shost); if (error) -- 2.43.7 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-29 11:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-29 8:53 [PATCH 0/2] Drop SCSI core dev->dma_mask check John Garry 2026-06-29 8:53 ` [PATCH 1/2] dma-mapping: make dma_max_mapping_size() return 0 for no DMA capability John Garry 2026-06-29 9:10 ` sashiko-bot 2026-06-29 10:11 ` Robin Murphy 2026-06-29 10:46 ` John Garry 2026-06-29 11:09 ` Robin Murphy 2026-06-29 8:53 ` [PATCH 2/2] scsi: core: Drop dev->dma_mask check in evaluating max_sectors John Garry
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.