* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-11-02 7:31 ` Sheng Yang
@ 2010-11-02 7:46 ` Jan Kiszka
2010-11-02 7:57 ` Sheng Yang
2010-11-02 8:00 ` Sheng Yang
2010-11-14 9:18 ` Jan Kiszka
2 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2010-11-02 7:46 UTC (permalink / raw)
To: Sheng Yang
Cc: Linux Kernel Mailing List, kvm, Avi Kivity, Marcelo Tosatti,
iommu, David Woodhouse
[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]
Am 02.11.2010 08:31, Sheng Yang wrote:
> On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Obtail the new pgd pointer before releasing the page containing this
>> value.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Who is taking care of this? The kvm tree?
>>
>> drivers/pci/intel-iommu.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
>> index 4789f8e..35463dd 100644
>> --- a/drivers/pci/intel-iommu.c
>> +++ b/drivers/pci/intel-iommu.c
>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
>> iommu_domain *domain,
>>
>> pte = dmar_domain->pgd;
>> if (dma_pte_present(pte)) {
>> - free_pgtable_page(dmar_domain->pgd);
>> dmar_domain->pgd = (struct dma_pte *)
>> phys_to_virt(dma_pte_addr(pte));
>> + free_pgtable_page(pte);
>> }
>> dmar_domain->agaw--;
>> }
>
> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>
> CC iommu mailing list and David.
>
> OK, Jan, I got your meaning now. And it's not the exactly swap. :)
>
> I think the old code is safe, seems it's broken(exposed) by:
>
> commit 1a8bd481bfba30515b54368d90a915db3faf302f
> Author: David Woodhouse <David.Woodhouse@intel.com>
> Date: Tue Aug 10 01:38:53 2010 +0100
>
> intel-iommu: Fix 32-bit build warning with __cmpxchg()
>
> drivers/pci/intel-iommu.c: In function 'dma_pte_addr':
> drivers/pci/intel-iommu.c:239: warning: passing argument 1 of '__cmpxchg64'
> from incompatible pointer typ
>
> It seems that __cmpxchg64() now cares about the type of its pointer argument,
> so give it a (uint64_t *) instead of a pointer to a structure which contains
> only that.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index c9171be..603cdc0 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -236,7 +236,7 @@ static inline u64 dma_pte_addr(struct dma_pte *pte)
> return pte->val & VTD_PAGE_MASK;
> #else
> /* Must have a full atomic 64-bit read */
> - return __cmpxchg64(pte, 0ULL, 0ULL) & VTD_PAGE_MASK;
> + return __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK;
> #endif
> }
>
> Seems here is the only affected code?
CONFIG_64BIT is on here, so this change did not make a difference for me.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-11-02 7:46 ` Jan Kiszka
@ 2010-11-02 7:57 ` Sheng Yang
0 siblings, 0 replies; 14+ messages in thread
From: Sheng Yang @ 2010-11-02 7:57 UTC (permalink / raw)
To: Jan Kiszka
Cc: Linux Kernel Mailing List, kvm, Avi Kivity, Marcelo Tosatti,
iommu, David Woodhouse
On Tuesday 02 November 2010 15:46:11 Jan Kiszka wrote:
> Am 02.11.2010 08:31, Sheng Yang wrote:
> > On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Obtail the new pgd pointer before releasing the page containing this
> >> value.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>
> >> Who is taking care of this? The kvm tree?
> >>
> >> drivers/pci/intel-iommu.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> >> index 4789f8e..35463dd 100644
> >> --- a/drivers/pci/intel-iommu.c
> >> +++ b/drivers/pci/intel-iommu.c
> >> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
> >> iommu_domain *domain,
> >>
> >> pte = dmar_domain->pgd;
> >> if (dma_pte_present(pte)) {
> >>
> >> - free_pgtable_page(dmar_domain->pgd);
> >>
> >> dmar_domain->pgd = (struct dma_pte *)
> >>
> >> phys_to_virt(dma_pte_addr(pte));
> >>
> >> + free_pgtable_page(pte);
> >>
> >> }
> >> dmar_domain->agaw--;
> >>
> >> }
> >
> > Reviewed-by: Sheng Yang <sheng@linux.intel.com>
> >
> > CC iommu mailing list and David.
> >
> > OK, Jan, I got your meaning now. And it's not the exactly swap. :)
> >
> > I think the old code is safe, seems it's broken(exposed) by:
> >
> > commit 1a8bd481bfba30515b54368d90a915db3faf302f
> > Author: David Woodhouse <David.Woodhouse@intel.com>
> > Date: Tue Aug 10 01:38:53 2010 +0100
> >
> > intel-iommu: Fix 32-bit build warning with __cmpxchg()
> >
> > drivers/pci/intel-iommu.c: In function 'dma_pte_addr':
> > drivers/pci/intel-iommu.c:239: warning: passing argument 1 of
> > '__cmpxchg64'
> >
> > from incompatible pointer typ
> >
> > It seems that __cmpxchg64() now cares about the type of its pointer
> > argument, so give it a (uint64_t *) instead of a pointer to a
> > structure which contains only that.
> >
> > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> >
> > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > index c9171be..603cdc0 100644
> > --- a/drivers/pci/intel-iommu.c
> > +++ b/drivers/pci/intel-iommu.c
> > @@ -236,7 +236,7 @@ static inline u64 dma_pte_addr(struct dma_pte *pte)
> >
> > return pte->val & VTD_PAGE_MASK;
> >
> > #else
> >
> > /* Must have a full atomic 64-bit read */
> >
> > - return __cmpxchg64(pte, 0ULL, 0ULL) & VTD_PAGE_MASK;
> > + return __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK;
> >
> > #endif
> > }
> >
> > Seems here is the only affected code?
>
> CONFIG_64BIT is on here, so this change did not make a difference for me.
Oh...
Then it would due to most VT-d machine wouldn't run into while (iommu->agaw <
dmar_domain->agaw).
We have routing test for VT-d devices assignment, but seems we don't use this kind
of VT-d machine for testing.
--
regards
Yang, Sheng
>
> Jan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-11-02 7:31 ` Sheng Yang
2010-11-02 7:46 ` Jan Kiszka
@ 2010-11-02 8:00 ` Sheng Yang
2010-11-14 9:18 ` Jan Kiszka
2 siblings, 0 replies; 14+ messages in thread
From: Sheng Yang @ 2010-11-02 8:00 UTC (permalink / raw)
To: kvm
Cc: Jan Kiszka, Linux Kernel Mailing List, Avi Kivity,
Marcelo Tosatti, iommu, David Woodhouse
On Tuesday 02 November 2010 15:31:22 Sheng Yang wrote:
> On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > Obtail the new pgd pointer before releasing the page containing this
> > value.
> >
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >
> > Who is taking care of this? The kvm tree?
> >
> > drivers/pci/intel-iommu.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > index 4789f8e..35463dd 100644
> > --- a/drivers/pci/intel-iommu.c
> > +++ b/drivers/pci/intel-iommu.c
> > @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
> > iommu_domain *domain,
> >
> > pte = dmar_domain->pgd;
> > if (dma_pte_present(pte)) {
> >
> > - free_pgtable_page(dmar_domain->pgd);
> >
> > dmar_domain->pgd = (struct dma_pte *)
> >
> > phys_to_virt(dma_pte_addr(pte));
> >
> > + free_pgtable_page(pte);
> >
> > }
> > dmar_domain->agaw--;
> >
> > }
>
> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>
> CC iommu mailing list and David.
>
> OK, Jan, I got your meaning now. And it's not the exactly swap. :)
>
> I think the old code is safe, seems it's broken(exposed) by:
>
> commit 1a8bd481bfba30515b54368d90a915db3faf302f
> Author: David Woodhouse <David.Woodhouse@intel.com>
> Date: Tue Aug 10 01:38:53 2010 +0100
>
> intel-iommu: Fix 32-bit build warning with __cmpxchg()
In fact this one shouldn't affect the result. Wrong guess...
--
regards
Yang, Sheng
>
> drivers/pci/intel-iommu.c: In function 'dma_pte_addr':
> drivers/pci/intel-iommu.c:239: warning: passing argument 1 of
> '__cmpxchg64' from incompatible pointer typ
>
> It seems that __cmpxchg64() now cares about the type of its pointer
> argument, so give it a (uint64_t *) instead of a pointer to a structure
> which contains only that.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index c9171be..603cdc0 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -236,7 +236,7 @@ static inline u64 dma_pte_addr(struct dma_pte *pte)
> return pte->val & VTD_PAGE_MASK;
> #else
> /* Must have a full atomic 64-bit read */
> - return __cmpxchg64(pte, 0ULL, 0ULL) & VTD_PAGE_MASK;
> + return __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK;
> #endif
> }
>
> Seems here is the only affected code?
>
> --
> regards
> Yang, Sheng
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-11-02 7:31 ` Sheng Yang
2010-11-02 7:46 ` Jan Kiszka
2010-11-02 8:00 ` Sheng Yang
@ 2010-11-14 9:18 ` Jan Kiszka
2010-12-10 8:36 ` Jan Kiszka
2 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2010-11-14 9:18 UTC (permalink / raw)
To: Sheng Yang, David Woodhouse
Cc: Linux Kernel Mailing List, kvm, Avi Kivity, Marcelo Tosatti, iommu
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
Am 02.11.2010 08:31, Sheng Yang wrote:
> On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Obtail the new pgd pointer before releasing the page containing this
>> value.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Who is taking care of this? The kvm tree?
>>
>> drivers/pci/intel-iommu.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
>> index 4789f8e..35463dd 100644
>> --- a/drivers/pci/intel-iommu.c
>> +++ b/drivers/pci/intel-iommu.c
>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
>> iommu_domain *domain,
>>
>> pte = dmar_domain->pgd;
>> if (dma_pte_present(pte)) {
>> - free_pgtable_page(dmar_domain->pgd);
>> dmar_domain->pgd = (struct dma_pte *)
>> phys_to_virt(dma_pte_addr(pte));
>> + free_pgtable_page(pte);
>> }
>> dmar_domain->agaw--;
>> }
>
> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>
> CC iommu mailing list and David.
Ping...
I think this fix also qualifies for stable (.35 and .36).
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-11-14 9:18 ` Jan Kiszka
@ 2010-12-10 8:36 ` Jan Kiszka
2010-12-10 18:44 ` Chris Wright
0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2010-12-10 8:36 UTC (permalink / raw)
To: Sheng Yang, David Woodhouse
Cc: Linux Kernel Mailing List, kvm, Avi Kivity, Marcelo Tosatti, iommu
Am 14.11.2010 10:18, Jan Kiszka wrote:
> Am 02.11.2010 08:31, Sheng Yang wrote:
>> On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Obtail the new pgd pointer before releasing the page containing this
>>> value.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> Who is taking care of this? The kvm tree?
>>>
>>> drivers/pci/intel-iommu.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
>>> index 4789f8e..35463dd 100644
>>> --- a/drivers/pci/intel-iommu.c
>>> +++ b/drivers/pci/intel-iommu.c
>>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
>>> iommu_domain *domain,
>>>
>>> pte = dmar_domain->pgd;
>>> if (dma_pte_present(pte)) {
>>> - free_pgtable_page(dmar_domain->pgd);
>>> dmar_domain->pgd = (struct dma_pte *)
>>> phys_to_virt(dma_pte_addr(pte));
>>> + free_pgtable_page(pte);
>>> }
>>> dmar_domain->agaw--;
>>> }
>>
>> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>>
>> CC iommu mailing list and David.
>
> Ping...
>
> I think this fix also qualifies for stable (.35 and .36).
>
Still not merged?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-12-10 8:36 ` Jan Kiszka
@ 2010-12-10 18:44 ` Chris Wright
2011-01-04 10:42 ` Jan Kiszka
0 siblings, 1 reply; 14+ messages in thread
From: Chris Wright @ 2010-12-10 18:44 UTC (permalink / raw)
To: Jan Kiszka
Cc: Sheng Yang, David Woodhouse, iommu, Marcelo Tosatti,
Linux Kernel Mailing List, kvm, Avi Kivity
* Jan Kiszka (jan.kiszka@siemens.com) wrote:
> >>> --- a/drivers/pci/intel-iommu.c
> >>> +++ b/drivers/pci/intel-iommu.c
> >>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
> >>> iommu_domain *domain,
> >>>
> >>> pte = dmar_domain->pgd;
> >>> if (dma_pte_present(pte)) {
> >>> - free_pgtable_page(dmar_domain->pgd);
> >>> dmar_domain->pgd = (struct dma_pte *)
> >>> phys_to_virt(dma_pte_addr(pte));
While here, might as well remove the unnecessary cast.
> >>> + free_pgtable_page(pte);
> >>> }
> >>> dmar_domain->agaw--;
> >>> }
> >>
> >> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
> >> CC iommu mailing list and David.
> >
> > Ping...
> >
> > I think this fix also qualifies for stable (.35 and .36).
> >
>
> Still not merged?
David, do you plan to pick this one up?
thanks,
-chris
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] intel-iommu: Fix use after release during device attach
2010-12-10 18:44 ` Chris Wright
@ 2011-01-04 10:42 ` Jan Kiszka
2011-04-21 12:32 ` [PATCH v2] " Jan Kiszka
0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2011-01-04 10:42 UTC (permalink / raw)
To: Chris Wright, David Woodhouse
Cc: Sheng Yang, David Woodhouse, iommu, Marcelo Tosatti,
Linux Kernel Mailing List, kvm, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
Am 10.12.2010 19:44, Chris Wright wrote:
> * Jan Kiszka (jan.kiszka@siemens.com) wrote:
>>>>> --- a/drivers/pci/intel-iommu.c
>>>>> +++ b/drivers/pci/intel-iommu.c
>>>>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
>>>>> iommu_domain *domain,
>>>>>
>>>>> pte = dmar_domain->pgd;
>>>>> if (dma_pte_present(pte)) {
>>>>> - free_pgtable_page(dmar_domain->pgd);
>>>>> dmar_domain->pgd = (struct dma_pte *)
>>>>> phys_to_virt(dma_pte_addr(pte));
>
> While here, might as well remove the unnecessary cast.
>
>>>>> + free_pgtable_page(pte);
>>>>> }
>>>>> dmar_domain->agaw--;
>>>>> }
>>>>
>>>> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>
> Acked-by: Chris Wright <chrisw@sous-sol.org>
>
>>>> CC iommu mailing list and David.
>>>
>>> Ping...
>>>
>>> I think this fix also qualifies for stable (.35 and .36).
>>>
>>
>> Still not merged?
>
> David, do you plan to pick this one up?
>
> thanks,
> -chris
Hmm, still no reaction. Trying David's Intel address now...
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2] intel-iommu: Fix use after release during device attach
2011-01-04 10:42 ` Jan Kiszka
@ 2011-04-21 12:32 ` Jan Kiszka
2011-04-21 14:02 ` Chris Wright
2011-04-21 14:28 ` Alex Williamson
0 siblings, 2 replies; 14+ messages in thread
From: Jan Kiszka @ 2011-04-21 12:32 UTC (permalink / raw)
To: Chris Wright, David Woodhouse
Cc: Sheng Yang, David Woodhouse, iommu, Marcelo Tosatti,
Linux Kernel Mailing List, kvm, Avi Kivity
On 2011-01-04 11:42, Jan Kiszka wrote:
> Am 10.12.2010 19:44, Chris Wright wrote:
>> * Jan Kiszka (jan.kiszka@siemens.com) wrote:
>>>>>> --- a/drivers/pci/intel-iommu.c
>>>>>> +++ b/drivers/pci/intel-iommu.c
>>>>>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
>>>>>> iommu_domain *domain,
>>>>>>
>>>>>> pte = dmar_domain->pgd;
>>>>>> if (dma_pte_present(pte)) {
>>>>>> - free_pgtable_page(dmar_domain->pgd);
>>>>>> dmar_domain->pgd = (struct dma_pte *)
>>>>>> phys_to_virt(dma_pte_addr(pte));
>>
>> While here, might as well remove the unnecessary cast.
>>
>>>>>> + free_pgtable_page(pte);
>>>>>> }
>>>>>> dmar_domain->agaw--;
>>>>>> }
>>>>>
>>>>> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
>>
>> Acked-by: Chris Wright <chrisw@sous-sol.org>
>>
>>>>> CC iommu mailing list and David.
>>>>
>>>> Ping...
>>>>
>>>> I think this fix also qualifies for stable (.35 and .36).
>>>>
>>>
>>> Still not merged?
>>
>> David, do you plan to pick this one up?
>>
>> thanks,
>> -chris
>
> Hmm, still no reaction. Trying David's Intel address now...
>
> Jan
>
Walking through my old queues, I came across this one again.
Given the still lacking reaction from the official maintainer, I'm a
bit confused about the state of intel-iommu. Is it unmaintained? Should
this bug fix better be routed through the KVM tree as its only in-tree
user? Please enlighten me.
Note that the patch became stable material for 35..38 in the meantime,
and it should go into 39 before release as well.
Thanks,
Jan
-------8<--------
Obtain the new pgd pointer before releasing the page containing this
value. Remove unneeded cast at this chance as well.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/pci/intel-iommu.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
v1->v2: Clean up cast as suggested by Chris.
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 505c1c7..b3e5c43 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -3607,9 +3607,8 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
pte = dmar_domain->pgd;
if (dma_pte_present(pte)) {
- free_pgtable_page(dmar_domain->pgd);
- dmar_domain->pgd = (struct dma_pte *)
- phys_to_virt(dma_pte_addr(pte));
+ dmar_domain->pgd = phys_to_virt(dma_pte_addr(pte));
+ free_pgtable_page(pte);
}
dmar_domain->agaw--;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] intel-iommu: Fix use after release during device attach
2011-04-21 12:32 ` [PATCH v2] " Jan Kiszka
@ 2011-04-21 14:02 ` Chris Wright
2011-04-21 14:28 ` Alex Williamson
1 sibling, 0 replies; 14+ messages in thread
From: Chris Wright @ 2011-04-21 14:02 UTC (permalink / raw)
To: Jan Kiszka
Cc: Chris Wright, David Woodhouse, Sheng Yang, David Woodhouse,
iommu, Marcelo Tosatti, Linux Kernel Mailing List, kvm,
Avi Kivity
* Jan Kiszka (jan.kiszka@siemens.com) wrote:
> On 2011-01-04 11:42, Jan Kiszka wrote:
> > Am 10.12.2010 19:44, Chris Wright wrote:
> >> * Jan Kiszka (jan.kiszka@siemens.com) wrote:
> >>>>>> --- a/drivers/pci/intel-iommu.c
> >>>>>> +++ b/drivers/pci/intel-iommu.c
> >>>>>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
> >>>>>> iommu_domain *domain,
> >>>>>>
> >>>>>> pte = dmar_domain->pgd;
> >>>>>> if (dma_pte_present(pte)) {
> >>>>>> - free_pgtable_page(dmar_domain->pgd);
> >>>>>> dmar_domain->pgd = (struct dma_pte *)
> >>>>>> phys_to_virt(dma_pte_addr(pte));
> >>
> >> While here, might as well remove the unnecessary cast.
> >>
> >>>>>> + free_pgtable_page(pte);
> >>>>>> }
> >>>>>> dmar_domain->agaw--;
> >>>>>> }
> >>>>>
> >>>>> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
> >>
> >> Acked-by: Chris Wright <chrisw@sous-sol.org>
> >>
> >>>>> CC iommu mailing list and David.
> >>>>
> >>>> Ping...
> >>>>
> >>>> I think this fix also qualifies for stable (.35 and .36).
> >>>>
> >>>
> >>> Still not merged?
> >>
> >> David, do you plan to pick this one up?
> >>
> >> thanks,
> >> -chris
> >
> > Hmm, still no reaction. Trying David's Intel address now...
> >
> > Jan
> >
>
> Walking through my old queues, I came across this one again.
>
> Given the still lacking reaction from the official maintainer, I'm a
> bit confused about the state of intel-iommu. Is it unmaintained? Should
> this bug fix better be routed through the KVM tree as its only in-tree
> user? Please enlighten me.
>
> Note that the patch became stable material for 35..38 in the meantime,
> and it should go into 39 before release as well.
>
> Thanks,
> Jan
>
> -------8<--------
>
> Obtain the new pgd pointer before releasing the page containing this
> value. Remove unneeded cast at this chance as well.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
> ---
> drivers/pci/intel-iommu.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> v1->v2: Clean up cast as suggested by Chris.
>
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index 505c1c7..b3e5c43 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -3607,9 +3607,8 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
>
> pte = dmar_domain->pgd;
> if (dma_pte_present(pte)) {
> - free_pgtable_page(dmar_domain->pgd);
> - dmar_domain->pgd = (struct dma_pte *)
> - phys_to_virt(dma_pte_addr(pte));
> + dmar_domain->pgd = phys_to_virt(dma_pte_addr(pte));
> + free_pgtable_page(pte);
> }
> dmar_domain->agaw--;
> }
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] intel-iommu: Fix use after release during device attach
2011-04-21 12:32 ` [PATCH v2] " Jan Kiszka
2011-04-21 14:02 ` Chris Wright
@ 2011-04-21 14:28 ` Alex Williamson
2011-04-21 15:42 ` David Woodhouse
1 sibling, 1 reply; 14+ messages in thread
From: Alex Williamson @ 2011-04-21 14:28 UTC (permalink / raw)
To: Jan Kiszka
Cc: Chris Wright, David Woodhouse, kvm, Marcelo Tosatti,
Linux Kernel Mailing List, iommu, Avi Kivity, David Woodhouse
On Thu, 2011-04-21 at 14:32 +0200, Jan Kiszka wrote:
> On 2011-01-04 11:42, Jan Kiszka wrote:
> > Am 10.12.2010 19:44, Chris Wright wrote:
> >> * Jan Kiszka (jan.kiszka@siemens.com) wrote:
> >>>>>> --- a/drivers/pci/intel-iommu.c
> >>>>>> +++ b/drivers/pci/intel-iommu.c
> >>>>>> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct
> >>>>>> iommu_domain *domain,
> >>>>>>
> >>>>>> pte = dmar_domain->pgd;
> >>>>>> if (dma_pte_present(pte)) {
> >>>>>> - free_pgtable_page(dmar_domain->pgd);
> >>>>>> dmar_domain->pgd = (struct dma_pte *)
> >>>>>> phys_to_virt(dma_pte_addr(pte));
> >>
> >> While here, might as well remove the unnecessary cast.
> >>
> >>>>>> + free_pgtable_page(pte);
> >>>>>> }
> >>>>>> dmar_domain->agaw--;
> >>>>>> }
> >>>>>
> >>>>> Reviewed-by: Sheng Yang <sheng@linux.intel.com>
> >>
> >> Acked-by: Chris Wright <chrisw@sous-sol.org>
> >>
> >>>>> CC iommu mailing list and David.
> >>>>
> >>>> Ping...
> >>>>
> >>>> I think this fix also qualifies for stable (.35 and .36).
> >>>>
> >>>
> >>> Still not merged?
> >>
> >> David, do you plan to pick this one up?
> >>
> >> thanks,
> >> -chris
> >
> > Hmm, still no reaction. Trying David's Intel address now...
> >
> > Jan
> >
>
> Walking through my old queues, I came across this one again.
>
> Given the still lacking reaction from the official maintainer, I'm a
> bit confused about the state of intel-iommu. Is it unmaintained? Should
> this bug fix better be routed through the KVM tree as its only in-tree
> user? Please enlighten me.
I've been wondering the exact same thing. My last patch took weeks of
prodding, finally went into the maintainer's tree without
acknowledgment, and there's hardly been any activity there to suggest a
pull request for 2.6.39 is going to happen. David, are you still
interested in maintaining this code? Thanks,
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] intel-iommu: Fix use after release during device attach
2011-04-21 14:28 ` Alex Williamson
@ 2011-04-21 15:42 ` David Woodhouse
2011-04-21 16:14 ` Alex Williamson
0 siblings, 1 reply; 14+ messages in thread
From: David Woodhouse @ 2011-04-21 15:42 UTC (permalink / raw)
To: Alex Williamson
Cc: Jan Kiszka, Chris Wright, kvm, Marcelo Tosatti,
Linux Kernel Mailing List, iommu, Avi Kivity
On Thu, 2011-04-21 at 15:28 +0100, Alex Williamson wrote:
> I've been wondering the exact same thing. My last patch took weeks of
> prodding, finally went into the maintainer's tree without
> acknowledgment, and there's hardly been any activity there to suggest
> a pull request for 2.6.39 is going to happen. David, are you still
> interested in maintaining this code? Thanks,
Yes, sorry, I've been somewhat snowed under with various things.
This patch has been in my tree for a while, and I've just merged one
more patch which is outstanding and sent Linus a pull request.
--
dwmw2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] intel-iommu: Fix use after release during device attach
2011-04-21 15:42 ` David Woodhouse
@ 2011-04-21 16:14 ` Alex Williamson
0 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2011-04-21 16:14 UTC (permalink / raw)
To: David Woodhouse
Cc: Jan Kiszka, Chris Wright, kvm, Marcelo Tosatti,
Linux Kernel Mailing List, iommu, Avi Kivity
On Thu, 2011-04-21 at 16:42 +0100, David Woodhouse wrote:
> On Thu, 2011-04-21 at 15:28 +0100, Alex Williamson wrote:
> > I've been wondering the exact same thing. My last patch took weeks of
> > prodding, finally went into the maintainer's tree without
> > acknowledgment, and there's hardly been any activity there to suggest
> > a pull request for 2.6.39 is going to happen. David, are you still
> > interested in maintaining this code? Thanks,
>
> Yes, sorry, I've been somewhat snowed under with various things.
>
> This patch has been in my tree for a while, and I've just merged one
> more patch which is outstanding and sent Linus a pull request.
Thanks David, I know you've been busy lately.
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread