[PATCH resend] gpu: host1x: fix error return code in host1x_memory_context_list_init()

[PATCH resend] gpu: host1x: fix error return code in host1x_memory_context_list_init()

[Yang Yingliang]

If context device has no IOMMU, the 'cdl->devs' is freed in error path,
but host1x_memory_context_list_init() doesn't return an error code, so
the module can be loaded successfully, when it's unloading, the
host1x_memory_context_list_free() is called in host1x_remove(), it will
cause double free. Return an error code to fix this.

Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
The previous patch link:
https://lore.kernel.org/lkml/20220714031123.2154506-1-yangyingliang@huawei.com/
---
 drivers/gpu/host1x/context.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c
index b08cf11f9a66..5cf9b98bedd0 100644
--- a/drivers/gpu/host1x/context.c
+++ b/drivers/gpu/host1x/context.c
@@ -74,6 +74,7 @@ int host1x_memory_context_list_init(struct host1x *host1x)
 		if (!fwspec || !device_iommu_mapped(&ctx->dev)) {
 			dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i);
 			device_del(&ctx->dev);
+			err = -EOPNOTSUPP;
 			goto del_devices;
 		}
 
-- 
2.25.1

Re: [PATCH resend] gpu: host1x: fix error return code in host1x_memory_context_list_init()

[Mikko Perttunen]

On 11/24/22 10:05, Yang Yingliang wrote:
> If context device has no IOMMU, the 'cdl->devs' is freed in error path,
> but host1x_memory_context_list_init() doesn't return an error code, so
> the module can be loaded successfully, when it's unloading, the
> host1x_memory_context_list_free() is called in host1x_remove(), it will
> cause double free. Return an error code to fix this.
> 
> Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> The previous patch link:
> https://lore.kernel.org/lkml/20220714031123.2154506-1-yangyingliang@huawei.com/
> ---
>   drivers/gpu/host1x/context.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c
> index b08cf11f9a66..5cf9b98bedd0 100644
> --- a/drivers/gpu/host1x/context.c
> +++ b/drivers/gpu/host1x/context.c
> @@ -74,6 +74,7 @@ int host1x_memory_context_list_init(struct host1x *host1x)
>   		if (!fwspec || !device_iommu_mapped(&ctx->dev)) {
>   			dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i);
>   			device_del(&ctx->dev);
> +			err = -EOPNOTSUPP;
>   			goto del_devices;
>   		}
>   

One side effect of this patch would be that if IOMMU is disabled on a 
system with context devices defined, Host1x won't work at all (currently 
probe continues and it works though without context isolation).

I'm not sure if that's something anyone is likely to run into, but it 
might be better to get rid of one of the frees instead.

If you can update with that that'd be great, or I can put it onto the 
TODO list.

Thanks,
Mikko

Re: [PATCH resend] gpu: host1x: fix error return code in host1x_memory_context_list_init()

[Yang Yingliang]

On 2022/11/26 0:00, Mikko Perttunen wrote:
> On 11/24/22 10:05, Yang Yingliang wrote:
>> If context device has no IOMMU, the 'cdl->devs' is freed in error path,
>> but host1x_memory_context_list_init() doesn't return an error code, so
>> the module can be loaded successfully, when it's unloading, the
>> host1x_memory_context_list_free() is called in host1x_remove(), it will
>> cause double free. Return an error code to fix this.
>>
>> Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>> The previous patch link:
>> https://lore.kernel.org/lkml/20220714031123.2154506-1-yangyingliang@huawei.com/ 
>>
>> ---
>>   drivers/gpu/host1x/context.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c
>> index b08cf11f9a66..5cf9b98bedd0 100644
>> --- a/drivers/gpu/host1x/context.c
>> +++ b/drivers/gpu/host1x/context.c
>> @@ -74,6 +74,7 @@ int host1x_memory_context_list_init(struct host1x 
>> *host1x)
>>           if (!fwspec || !device_iommu_mapped(&ctx->dev)) {
>>               dev_err(host1x->dev, "Context device %d has no 
>> IOMMU!\n", i);
>>               device_del(&ctx->dev);
>> +            err = -EOPNOTSUPP;
>>               goto del_devices;
>>           }
>
> One side effect of this patch would be that if IOMMU is disabled on a 
> system with context devices defined, Host1x won't work at all 
> (currently probe continues and it works though without context 
> isolation).
>
> I'm not sure if that's something anyone is likely to run into, but it 
> might be better to get rid of one of the frees instead.
>
> If you can update with that that'd be great, or I can put it onto the 
> TODO list.
I can send a v2 later.

Thanks,
Yang
>
> Thanks,
> Mikko
> .