On Mon, Nov 07, 2016 at 03:39:02PM +0000, Robin Murphy wrote:
> When we iterate a master's config entries, what we generally care
> about is the entry's stream map index, rather than the entry index
> itself, so it's nice to have the iterator automatically assign the
> former from the latter. Unfortunately, booting with KASAN reveals
> the oversight that using a simple comma operator results in the
> entry index being dereferenced before being checked for validity,
> so we always access one element past the end of the fwspec array.
>
> Flip things around so that the check always happens before the index
> may be dereferenced.
>
> Fixes: adfec2e709d2 ("iommu/arm-smmu: Convert to iommu_fwspec")
> Reported-by: Mark Rutland <[email protected]>
> Signed-off-by: Robin Murphy <[email protected]>
> ---
> drivers/iommu/arm-smmu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f86683eec446..44ffe3a391d6 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -324,8 +324,10 @@ struct arm_smmu_master_cfg {
> #define INVALID_SMENDX -1
> #define __fwspec_cfg(fw) ((struct arm_smmu_master_cfg *)fw->iommu_priv)
> #define fwspec_smmu(fw) (__fwspec_cfg(fw)->smmu)
> -#define for_each_cfg_sme(fw, i, idx) \
> - for (i = 0; idx = __fwspec_cfg(fw)->smendx[i], i < fw->num_ids; ++i)
> +#define for_each_cfg_sme(fw, i, idx) \
> + for (i = 0; \
> + i < fw->num_ids && (idx = __fwspec_cfg(fw)->smendx[i], true); \
> + ++i)
Urgh, that's vile. Is it worth wrapping that in (yet another) macro, which
either returns the index or -ENOENT if i is out of bounds?
Will
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
