Re: [SPAM][PATCH] iommu/mediatek: Validate number of phandles associated with "mediatek,larbs"

[Yong Wu <yong.wu@mediatek.com>]

On Fri, 2021-12-10 at 12:57 -0800, Guenter Roeck wrote:
> Since commit baf94e6ebff9 ("iommu/mediatek: Add device link for smi-
> common
> and m4u"), the driver assumes that at least one phandle associated
> with
> "mediatek,larbs" exists. If that is not the case, for example if
> reason
> "mediatek,larbs" is provided as boolean property, the code will use
> an
> uninitialized pointer and may crash. To fix the problem, ensure that
> the
> number of phandles associated with "mediatek,larbs" is at least 1 and
> bail out immediately if that is not the case.

From the dt-binding, "mediatek,larbs" always is a phandle-array. I 
assumed the dts should conform to the dt-binding before. Then the
problem is that if we should cover the case that someone abuses/attacks
the dts. Could you help add more comment in the commit message?
something like: this is for avoid abuse the dt-binding.

> 
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Tomasz Figa <tfiga@chromium.org>
> Fixes: baf94e6ebff9 ("iommu/mediatek: Add device link for smi-common
> and m4u")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/iommu/mtk_iommu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 25b834104790..0bbe32d0a2a6 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -828,6 +828,8 @@ static int mtk_iommu_probe(struct platform_device
> *pdev)
>  					     "mediatek,larbs", NULL);
>  	if (larb_nr < 0)
>  		return larb_nr;
> +	if (larb_nr == 0)
> +		return -EINVAL;

Just assigning the larbnode to NULL may be simpler. In this case, it
won't enter the loop below, and return 0 in the
of_parse_phandle(larbnode, "mediatek,smi", 0).

-       struct device_node      *larbnode, *smicomm_node;
+       struct device_node      *larbnode = NULL, *smicomm_node;

>  
>  	for (i = 0; i < larb_nr; i++) {
>  		u32 id;
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek