[PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[Dan Carpenter]

The apple_dart_domain_alloc_paging() function is supposed to return NULL
on error.  Returning an error pointer will lead to an Oops in
__iommu_domain_alloc().

Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/iommu/apple-dart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index ee05f4824bfa..cb38a7a826dc 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -761,7 +761,7 @@ static struct iommu_domain *apple_dart_domain_alloc_paging(struct device *dev)
 		ret = apple_dart_finalize_domain(dart_domain, cfg);
 		if (ret) {
 			kfree(dart_domain);
-			return ERR_PTR(ret);
+			return NULL;
 		}
 	}
 	return &dart_domain->domain;
-- 
2.42.0

Re: [PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[Jason Gunthorpe]

On Mon, Oct 30, 2023 at 12:03:12PM +0300, Dan Carpenter wrote:
> The apple_dart_domain_alloc_paging() function is supposed to return NULL
> on error.  Returning an error pointer will lead to an Oops in
> __iommu_domain_alloc().
> 
> Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/iommu/apple-dart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Really need to fix this so the function does return ERR_PTR..

Thanks,
Jason

Re: [PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[Dan Carpenter]

On Mon, Oct 30, 2023 at 08:56:50AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 30, 2023 at 12:03:12PM +0300, Dan Carpenter wrote:
> > The apple_dart_domain_alloc_paging() function is supposed to return NULL
> > on error.  Returning an error pointer will lead to an Oops in
> > __iommu_domain_alloc().
> > 
> > Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  drivers/iommu/apple-dart.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> Really need to fix this so the function does return ERR_PTR..

It's called as a function pointer.  Changing that will create a
backporting hazard unless we rename the pointer or something.

regards,
dan carpenter

Re: [PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[Jason Gunthorpe]

On Mon, Oct 30, 2023 at 03:00:56PM +0300, Dan Carpenter wrote:
> On Mon, Oct 30, 2023 at 08:56:50AM -0300, Jason Gunthorpe wrote:
> > On Mon, Oct 30, 2023 at 12:03:12PM +0300, Dan Carpenter wrote:
> > > The apple_dart_domain_alloc_paging() function is supposed to return NULL
> > > on error.  Returning an error pointer will lead to an Oops in
> > > __iommu_domain_alloc().
> > > 
> > > Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > ---
> > >  drivers/iommu/apple-dart.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > 
> > Really need to fix this so the function does return ERR_PTR..
> 
> It's called as a function pointer.  Changing that will create a
> backporting hazard unless we rename the pointer or something.

You can make that argument about almost any change in the kernel..

IMHO we needed an annotation like __user/__iomem/etc to indicate
ERR_PTR so at least there is some hope of trivially finding it.

Jason

Re: [PATCH] iommu/dart: Fix return code in apple_dart_domain_alloc_paging()

[Jason Gunthorpe]

On Mon, Oct 30, 2023 at 09:38:23AM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 30, 2023 at 03:00:56PM +0300, Dan Carpenter wrote:
> > On Mon, Oct 30, 2023 at 08:56:50AM -0300, Jason Gunthorpe wrote:
> > > On Mon, Oct 30, 2023 at 12:03:12PM +0300, Dan Carpenter wrote:
> > > > The apple_dart_domain_alloc_paging() function is supposed to return NULL
> > > > on error.  Returning an error pointer will lead to an Oops in
> > > > __iommu_domain_alloc().
> > > > 
> > > > Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > > ---
> > > >  drivers/iommu/apple-dart.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > > 
> > > Really need to fix this so the function does return ERR_PTR..
> > 
> > It's called as a function pointer.  Changing that will create a
> > backporting hazard unless we rename the pointer or something.
> 
> You can make that argument about almost any change in the kernel..
> 
> IMHO we needed an annotation like __user/__iomem/etc to indicate
> ERR_PTR so at least there is some hope of trivially finding it.

I was thinking about this some more, I think I prefer we fix the core
code to accept the ERR_PTR and just start moving gently in that
direction. I was working on some other stuff and it is starting to get
really confusing that only this op is different.

I'll send a patch

Jason