2019-07-22 17:28:38

by Eric Auger

Subject: [PATCH 1/2] dma-mapping: Protect dma_addressing_limited against NULL dma_mask

dma_addressing_limited() should not be called on a device with
a NULL dma_mask. If this occurs let's WARN_ON_ONCE and immediately
return. Existing call sites are updated separately.

Fixes: b866455423e0 ("dma-mapping: add a dma_addressing_limited helper")
Signed-off-by: Eric Auger <[email protected]>

---

v1 -> v2:
- add a WARN_ON_ONCE()
- reword the commit message
---
include/linux/dma-mapping.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e11b115dd0e4..ef0cf9537abc 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -689,8 +689,9 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask)
*/
static inline bool dma_addressing_limited(struct device *dev)
{
- return min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
- dma_get_required_mask(dev);
+ return WARN_ON_ONCE(!dev->dma_mask) ? false :
+ min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
+ dma_get_required_mask(dev);
}

#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
--
2.20.1

2019-07-22 17:31:10

by Christoph Hellwig

Subject: Re: [PATCH 1/2] dma-mapping: Protect dma_addressing_limited against NULL dma_mask

> static inline bool dma_addressing_limited(struct device *dev)
> {
> - return min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
> - dma_get_required_mask(dev);
> + return WARN_ON_ONCE(!dev->dma_mask) ? false :
> + min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
> + dma_get_required_mask(dev);

This should really use a separate if statement, but I can fix that
up when applying it.

2019-07-22 19:47:42

by Eric Auger

Subject: Re: [PATCH 1/2] dma-mapping: Protect dma_addressing_limited against NULL dma_mask

Hi Christoph,

On 7/22/19 5:26 PM, Christoph Hellwig wrote:
>> static inline bool dma_addressing_limited(struct device *dev)
>> {
>> - return min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
>> - dma_get_required_mask(dev);
>> + return WARN_ON_ONCE(!dev->dma_mask) ? false :
>> + min_not_zero(*dev->dma_mask, dev->bus_dma_mask) <
>> + dma_get_required_mask(dev);
>
> This should really use a separate if statement, but I can fix that
> up when applying it.
>
Just wondering why we don't use the dma_get_mask() accessor which
returns DMA_BIT_MASK(32) in case the dma_mask is not set.

Do you foresee any issue and would it still mandate to add dma_mask
checks on each call sites?

Thanks

Eric