media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

[PATCH] media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

Posted by Dan Carpenter 3 months, 2 weeks ago

The iommu_paging_domain_alloc() function doesn't return NULL on error it
returns error pointers.  Update the check and then set ->empty_domain to
NULL because the rest of the driver assumes it can be NULL.

Fixes: ff8c5622f9f7 ("media: rkvdec: Restore iommu addresses on errors")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/staging/media/rkvdec/rkvdec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index d707088ec0dc..1b7f27e4d961 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -1162,8 +1162,10 @@ static int rkvdec_probe(struct platform_device *pdev)
 	if (iommu_get_domain_for_dev(&pdev->dev)) {
 		rkvdec->empty_domain = iommu_paging_domain_alloc(rkvdec->dev);
 
-		if (!rkvdec->empty_domain)
+		if (IS_ERR(rkvdec->empty_domain)) {
+			rkvdec->empty_domain = NULL;
 			dev_warn(rkvdec->dev, "cannot alloc new empty domain\n");
+		}
 	}
 
 	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
-- 
2.47.2

Re: [PATCH] media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

Posted by Nicolas Dufresne 1 month, 4 weeks ago

Hi Dan,

Le mercredi 25 juin 2025 à 10:23 -0500, Dan Carpenter a écrit :
> The iommu_paging_domain_alloc() function doesn't return NULL on error it
> returns error pointers.  Update the check and then set ->empty_domain to
> NULL because the rest of the driver assumes it can be NULL.
> 
> Fixes: ff8c5622f9f7 ("media: rkvdec: Restore iommu addresses on errors")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 4 +++-

I've re-applied to the now de-staged driver, and will send that as fixes for
6.17.

thanks,
Nicolas

>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c
> b/drivers/staging/media/rkvdec/rkvdec.c
> index d707088ec0dc..1b7f27e4d961 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -1162,8 +1162,10 @@ static int rkvdec_probe(struct platform_device *pdev)
>  	if (iommu_get_domain_for_dev(&pdev->dev)) {
>  		rkvdec->empty_domain = iommu_paging_domain_alloc(rkvdec-
> >dev);
>  
> -		if (!rkvdec->empty_domain)
> +		if (IS_ERR(rkvdec->empty_domain)) {
> +			rkvdec->empty_domain = NULL;
>  			dev_warn(rkvdec->dev, "cannot alloc new empty
> domain\n");
> +		}
>  	}
>  
>  	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

Re: [PATCH] media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

Posted by Detlev Casanova 2 months, 2 weeks ago

Hi Dan,

On Wednesday, 25 June 2025 11:23:10 EDT Dan Carpenter wrote:
> The iommu_paging_domain_alloc() function doesn't return NULL on error it
> returns error pointers.  Update the check and then set ->empty_domain to
> NULL because the rest of the driver assumes it can be NULL.
> 
> Fixes: ff8c5622f9f7 ("media: rkvdec: Restore iommu addresses on errors")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c
> b/drivers/staging/media/rkvdec/rkvdec.c index d707088ec0dc..1b7f27e4d961
> 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -1162,8 +1162,10 @@ static int rkvdec_probe(struct platform_device *pdev)
> if (iommu_get_domain_for_dev(&pdev->dev)) {
>  		rkvdec->empty_domain = 
iommu_paging_domain_alloc(rkvdec->dev);
> 
> -		if (!rkvdec->empty_domain)
> +		if (IS_ERR(rkvdec->empty_domain)) {
> +			rkvdec->empty_domain = NULL;
>  			dev_warn(rkvdec->dev, "cannot alloc new 
empty domain\n");
> +		}
>  	}
> 
>  	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

Thank you for finding this one. I tested it on rock 5b and all is good. 

Tested-by: Detlev Casanova <detlev.casanova@collabora.com>

Regards,
Detlev.

Re: [PATCH] media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

Posted by Nicolas Dufresne 3 months, 2 weeks ago

Hi,

Le mercredi 25 juin 2025 à 10:23 -0500, Dan Carpenter a écrit :
> The iommu_paging_domain_alloc() function doesn't return NULL on error it
> returns error pointers.  Update the check and then set ->empty_domain to
> NULL because the rest of the driver assumes it can be NULL.
>
> Fixes: ff8c5622f9f7 ("media: rkvdec: Restore iommu addresses on errors")

Oh, sorry about that, I'll will test your patch this week, but otherwise
looks good to me, fixing yet one more error path. I'll take the time
to test dropping the iommu node from the DT while at it, as I simply
don't remember if that was re-tested after that change.

regards,
Nicolas

> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index d707088ec0dc..1b7f27e4d961 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -1162,8 +1162,10 @@ static int rkvdec_probe(struct platform_device *pdev)
>  	if (iommu_get_domain_for_dev(&pdev->dev)) {
>  		rkvdec->empty_domain = iommu_paging_domain_alloc(rkvdec->dev);
>  
> -		if (!rkvdec->empty_domain)
> +		if (IS_ERR(rkvdec->empty_domain)) {
> +			rkvdec->empty_domain = NULL;
>  			dev_warn(rkvdec->dev, "cannot alloc new empty domain\n");
> +		}
>  	}
>  
>  	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

Re: [PATCH] media: rkvdec: Fix a NULL vs IS_ERR() bug in probe()

Posted by Dan Carpenter 2 months, 3 weeks ago

On Wed, Jun 25, 2025 at 12:13:20PM -0400, Nicolas Dufresne wrote:
> Hi,
> 
> Le mercredi 25 juin 2025 à 10:23 -0500, Dan Carpenter a écrit :
> > The iommu_paging_domain_alloc() function doesn't return NULL on error it
> > returns error pointers.  Update the check and then set ->empty_domain to
> > NULL because the rest of the driver assumes it can be NULL.
> >
> > Fixes: ff8c5622f9f7 ("media: rkvdec: Restore iommu addresses on errors")
> 
> Oh, sorry about that, I'll will test your patch this week, but otherwise
> looks good to me, fixing yet one more error path. I'll take the time
> to test dropping the iommu node from the DT while at it, as I simply
> don't remember if that was re-tested after that change.
> 
> regards,
> Nicolas
> 

Ping?

regards,
dan carpenter