* [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
@ 2022-06-27 8:55 Yang Yingliang
2022-07-15 7:49 ` Joerg Roedel
0 siblings, 1 reply; 5+ messages in thread
From: Yang Yingliang @ 2022-06-27 8:55 UTC (permalink / raw)
To: linux-kernel, iommu, iommu; +Cc: joro, will, yf.wang
cookie_alloc() is called by iommu_get_dma_cookie() and iommu_get_msi_cookie(),
but the mutex is only initialized in iommu_get_dma_cookie(), move mutex_init()
into cookie_alloc() to make sure the mutex will be initialized.
Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during iova_domain initialization")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/iommu/dma-iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 1910f4f1612b..e29157380c48 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -294,6 +294,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
if (cookie) {
INIT_LIST_HEAD(&cookie->msi_page_list);
cookie->type = type;
+ mutex_init(&cookie->mutex);
}
return cookie;
}
@@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
if (!domain->iova_cookie)
return -ENOMEM;
- mutex_init(&domain->iova_cookie->mutex);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
2022-06-27 8:55 [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie() Yang Yingliang
@ 2022-07-15 7:49 ` Joerg Roedel
2022-07-15 9:28 ` Robin Murphy
0 siblings, 1 reply; 5+ messages in thread
From: Joerg Roedel @ 2022-07-15 7:49 UTC (permalink / raw)
To: Yang Yingliang; +Cc: linux-kernel, iommu, iommu, will, yf.wang, Robin Murphy
Adding Robin.
On Mon, Jun 27, 2022 at 04:55:33PM +0800, Yang Yingliang wrote:
> cookie_alloc() is called by iommu_get_dma_cookie() and iommu_get_msi_cookie(),
> but the mutex is only initialized in iommu_get_dma_cookie(), move mutex_init()
> into cookie_alloc() to make sure the mutex will be initialized.
>
> Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during iova_domain initialization")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> drivers/iommu/dma-iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 1910f4f1612b..e29157380c48 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -294,6 +294,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
> if (cookie) {
> INIT_LIST_HEAD(&cookie->msi_page_list);
> cookie->type = type;
> + mutex_init(&cookie->mutex);
> }
> return cookie;
> }
> @@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
> if (!domain->iova_cookie)
> return -ENOMEM;
>
> - mutex_init(&domain->iova_cookie->mutex);
> return 0;
> }
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
2022-07-15 7:49 ` Joerg Roedel
@ 2022-07-15 9:28 ` Robin Murphy
2022-07-18 6:42 ` Yang Yingliang
0 siblings, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2022-07-15 9:28 UTC (permalink / raw)
To: Joerg Roedel, Yang Yingliang; +Cc: linux-kernel, iommu, iommu, will, yf.wang
On 2022-07-15 08:49, Joerg Roedel wrote:
> Adding Robin.
>
> On Mon, Jun 27, 2022 at 04:55:33PM +0800, Yang Yingliang wrote:
>> cookie_alloc() is called by iommu_get_dma_cookie() and iommu_get_msi_cookie(),
>> but the mutex is only initialized in iommu_get_dma_cookie(), move mutex_init()
>> into cookie_alloc() to make sure the mutex will be initialized.
The mutex is only used in iommu_dma_init_domain(), which is only called
by iommu_setup_dma_ops() for IOMMU_DOMAIN_DMA domains. How is there a
problem here?
Robin.
>> Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during iova_domain initialization")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>> drivers/iommu/dma-iommu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 1910f4f1612b..e29157380c48 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -294,6 +294,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>> if (cookie) {
>> INIT_LIST_HEAD(&cookie->msi_page_list);
>> cookie->type = type;
>> + mutex_init(&cookie->mutex);
>> }
>> return cookie;
>> }
>> @@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>> if (!domain->iova_cookie)
>> return -ENOMEM;
>>
>> - mutex_init(&domain->iova_cookie->mutex);
>> return 0;
>> }
>>
>> --
>> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
2022-07-15 9:28 ` Robin Murphy
@ 2022-07-18 6:42 ` Yang Yingliang
2022-08-01 19:48 ` Robin Murphy
0 siblings, 1 reply; 5+ messages in thread
From: Yang Yingliang @ 2022-07-18 6:42 UTC (permalink / raw)
To: Robin Murphy, Joerg Roedel; +Cc: linux-kernel, iommu, iommu, will, yf.wang
Hi,
On 2022/7/15 17:28, Robin Murphy wrote:
> On 2022-07-15 08:49, Joerg Roedel wrote:
>> Adding Robin.
>>
>> On Mon, Jun 27, 2022 at 04:55:33PM +0800, Yang Yingliang wrote:
>>> cookie_alloc() is called by iommu_get_dma_cookie() and
>>> iommu_get_msi_cookie(),
>>> but the mutex is only initialized in iommu_get_dma_cookie(), move
>>> mutex_init()
>>> into cookie_alloc() to make sure the mutex will be initialized.
>
> The mutex is only used in iommu_dma_init_domain(), which is only
> called by iommu_setup_dma_ops() for IOMMU_DOMAIN_DMA domains. How is
> there a problem here?
It's no problem now, but I thinks it's better to initialize the 'mutex'
in cookie_alloc() to make code stronger.
Thanks,
Yang
>
> Robin.
>
>>> Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during
>>> iova_domain initialization")
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>>> ---
>>> drivers/iommu/dma-iommu.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index 1910f4f1612b..e29157380c48 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -294,6 +294,7 @@ static struct iommu_dma_cookie
>>> *cookie_alloc(enum iommu_dma_cookie_type type)
>>> if (cookie) {
>>> INIT_LIST_HEAD(&cookie->msi_page_list);
>>> cookie->type = type;
>>> + mutex_init(&cookie->mutex);
>>> }
>>> return cookie;
>>> }
>>> @@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain
>>> *domain)
>>> if (!domain->iova_cookie)
>>> return -ENOMEM;
>>> - mutex_init(&domain->iova_cookie->mutex);
>>> return 0;
>>> }
>>> --
>>> 2.25.1
> .
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie()
2022-07-18 6:42 ` Yang Yingliang
@ 2022-08-01 19:48 ` Robin Murphy
0 siblings, 0 replies; 5+ messages in thread
From: Robin Murphy @ 2022-08-01 19:48 UTC (permalink / raw)
To: Yang Yingliang, Joerg Roedel; +Cc: linux-kernel, iommu, iommu, will, yf.wang
On 2022-07-18 07:42, Yang Yingliang wrote:
> Hi,
>
> On 2022/7/15 17:28, Robin Murphy wrote:
>> On 2022-07-15 08:49, Joerg Roedel wrote:
>>> Adding Robin.
>>>
>>> On Mon, Jun 27, 2022 at 04:55:33PM +0800, Yang Yingliang wrote:
>>>> cookie_alloc() is called by iommu_get_dma_cookie() and
>>>> iommu_get_msi_cookie(),
>>>> but the mutex is only initialized in iommu_get_dma_cookie(), move
>>>> mutex_init()
>>>> into cookie_alloc() to make sure the mutex will be initialized.
>>
>> The mutex is only used in iommu_dma_init_domain(), which is only
>> called by iommu_setup_dma_ops() for IOMMU_DOMAIN_DMA domains. How is
>> there a problem here?
> It's no problem now, but I thinks it's better to initialize the 'mutex'
> in cookie_alloc() to make code stronger.
Stronger against what, though? The sole reason this mutex exists at all
is as a temporary measure to protect the IOVA domain from concurrent
initialisation - I suppose in hindsight it might have made sense to
define it inside the union, but I don't see much point in churning that
now. I'd rather spend the time on continuing to get the
iommu_probe_device() path sorted out so that we don't have the whole
problematic driver-probe-time-replay mess in the first place and this
mutex can be reverted ASAP.
>>>> Fixes: ac9a5d522bb8 ("iommu/dma: Fix race condition during
>>>> iova_domain initialization")
>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
Please teach your robot to care about things that actually matter. All
it's "reporting" here is that it isn't clever enough to follow control
flow across multiple compilation units, otherwise it should have seen
the matching iommu_is_dma_domain() checks between __iommu_domain_alloc()
and iommu_setup_dma_ops(). Having to explain this is not a good use of
the kernel community's time and effort.
Thanks,
Robin.
>>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>>>> ---
>>>> drivers/iommu/dma-iommu.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index 1910f4f1612b..e29157380c48 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -294,6 +294,7 @@ static struct iommu_dma_cookie
>>>> *cookie_alloc(enum iommu_dma_cookie_type type)
>>>> if (cookie) {
>>>> INIT_LIST_HEAD(&cookie->msi_page_list);
>>>> cookie->type = type;
>>>> + mutex_init(&cookie->mutex);
>>>> }
>>>> return cookie;
>>>> }
>>>> @@ -311,7 +312,6 @@ int iommu_get_dma_cookie(struct iommu_domain
>>>> *domain)
>>>> if (!domain->iova_cookie)
>>>> return -ENOMEM;
>>>> - mutex_init(&domain->iova_cookie->mutex);
>>>> return 0;
>>>> }
>>>> --
>>>> 2.25.1
>> .
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-01 19:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 8:55 [PATCH -next] iommu/dma: Fix missing mutex_init() in iommu_get_msi_cookie() Yang Yingliang
2022-07-15 7:49 ` Joerg Roedel
2022-07-15 9:28 ` Robin Murphy
2022-07-18 6:42 ` Yang Yingliang
2022-08-01 19:48 ` Robin Murphy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).