Hi Geert,
Thank you for the patch.
On Friday, 27 July 2018 11:44:47 EEST Geert Uytterhoeven wrote:
> Add an upper bound check for the MID/RID value passed from DT via the
> DMA spec.
>
> This avoids writing to reserved bits in the DMARS registers in case of
> an out-of-range value in DT.
Is this really useful ? In the normal case, when information in DT is correct,
this will just add overhead. What do we really want to guard against ? If we
merge this change, how much further do we need to go ? What other values
provided in DT, such as reg addresses, do we need to validate them too ?
Functionally speaking this change does no harm, but it increases the kernel
size, add overhead at runtime, and only addresses a very limited range of
invalid DT issues.
> Suggested-by: Renesas BSP team via Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com> Signed-off-by: Geert Uytterhoeven
> <geert+rene...@glider.be>
> ---
> drivers/dma/sh/rcar-dmac.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 72572320208dbb9a..73cf1053bed90244 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1644,8 +1644,11 @@ static struct dma_chan *rcar_dmac_of_xlate(struct
> of_phandle_args *dma_spec, struct dma_chan *chan;
> dma_cap_mask_t mask;
>
> - if (dma_spec->args_count != 1)
> + if (dma_spec->args_count != 1 || dma_spec->args[0] > 0xff) {
> + pr_info("%s: invalid MID/RID 0x%x... for %pOF\n", __func__,
> + dma_spec->args[0], dma_spec->np);
> return NULL;
> + }
>
> /* Only slave DMA channels can be allocated via DT */
> dma_cap_zero(mask);
--
Regards,
Laurent Pinchart
