[PATCH 1/2] iommu/amd: Remove entry from the list before freeing it

[PATCH 1/2] iommu/amd: Remove entry from the list before freeing it

[Jan Vesely]

From: Jan Vesely <jan.vesely@amd.com>

Signed-off-by: Jan Vesely <jan.vesely@amd.com>
---
 drivers/iommu/amd_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 634f636..17c76f2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3288,8 +3288,10 @@ static void amd_iommu_put_dm_regions(struct device *dev,
 {
 	struct iommu_dm_region *entry, *next;
 
-	list_for_each_entry_safe(entry, next, head, list)
+	list_for_each_entry_safe(entry, next, head, list) {
+		list_del(&entry->list);
 		kfree(entry);
+	}
 }
 
 static const struct iommu_ops amd_iommu_ops = {
-- 
2.5.5

[PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Jan Vesely]

From: Jan Vesely <jan.vesely@amd.com>

Signed-off-by: Jan Vesely <jan.vesely@amd.com>
---
 drivers/iommu/amd_iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 17c76f2..4ff5e40 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3016,6 +3016,7 @@ static void protection_domain_free(struct protection_domain *domain)
 
 	del_domain_from_list(domain);
 
+	mutex_destroy(&domain->api_lock);
 	if (domain->id)
 		domain_id_free(domain->id);
 
-- 
2.5.5

Re: [PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Vesely, Jan]

On Sat, 2016-05-21 at 14:11 -0400, Jan Vesely wrote:
> From: Jan Vesely <jan.vesely@amd.com>
> 
> Signed-off-by: Jan Vesely <jan.vesely@amd.com>
> ---
>  drivers/iommu/amd_iommu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 17c76f2..4ff5e40 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3016,6 +3016,7 @@ static void protection_domain_free(struct
> protection_domain *domain)
>  
>  	del_domain_from_list(domain);
>  
> +	mutex_destroy(&domain->api_lock);
>  	if (domain->id)
>  		domain_id_free(domain->id);
>  

ping

Re: [PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[joro]

On Thu, Jun 09, 2016 at 03:48:44PM +0000, Vesely, Jan wrote:
> On Sat, 2016-05-21 at 14:11 -0400, Jan Vesely wrote:
> > From: Jan Vesely <jan.vesely@amd.com>
> > 
> > Signed-off-by: Jan Vesely <jan.vesely@amd.com>
> > ---
> >  drivers/iommu/amd_iommu.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> > index 17c76f2..4ff5e40 100644
> > --- a/drivers/iommu/amd_iommu.c
> > +++ b/drivers/iommu/amd_iommu.c
> > @@ -3016,6 +3016,7 @@ static void protection_domain_free(struct
> > protection_domain *domain)
> >  
> >  	del_domain_from_list(domain);
> >  
> > +	mutex_destroy(&domain->api_lock);
> >  	if (domain->id)
> >  		domain_id_free(domain->id);
> >  
> 
> ping

Your patches lack commit messages, please add a more detailed
description of what you are fixing and why. Please also add 'Fixes:'
tags when resubmitting. The changes itself look good to me.


	Joerg

Re: [PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Vesely, Jan]

On Wed, 2016-06-15 at 13:54 +0200, joro@8bytes.org wrote:
> On Thu, Jun 09, 2016 at 03:48:44PM +0000, Vesely, Jan wrote:
> > On Sat, 2016-05-21 at 14:11 -0400, Jan Vesely wrote:
> > > From: Jan Vesely <jan.vesely@amd.com>
> > > 
> > > Signed-off-by: Jan Vesely <jan.vesely@amd.com>
> > > ---
> > >  drivers/iommu/amd_iommu.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/iommu/amd_iommu.c
> > > b/drivers/iommu/amd_iommu.c
> > > index 17c76f2..4ff5e40 100644
> > > --- a/drivers/iommu/amd_iommu.c
> > > +++ b/drivers/iommu/amd_iommu.c
> > > @@ -3016,6 +3016,7 @@ static void protection_domain_free(struct
> > > protection_domain *domain)
> > >  
> > >  	del_domain_from_list(domain);
> > >  
> > > +	mutex_destroy(&domain->api_lock);
> > >  	if (domain->id)
> > >  		domain_id_free(domain->id);
> > >  
> > 
> > ping
> 
> Your patches lack commit messages, please add a more detailed
> description of what you are fixing and why. Please also add 'Fixes:'
> tags when resubmitting. The changes itself look good to me.

There are no specific bugs/oopses that these patches fix (that I'm
aware of). Both were found while I was familiarizing myself with the
code looking for memory corruption (which turned out to be [0]).
Either issue would be very difficult to hit in the wild and
mutex_destroy is empty unless you compile using CONFIG_DEBUG_MUTEXES.

Jan

[0] https://lkml.org/lkml/2016/3/7/1087

> 
> 
> 	Joerg
> 

Re: [PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Borislav Petkov]

On Wed, Jun 15, 2016 at 05:53:15PM +0000, Vesely, Jan wrote:
> There are no specific bugs/oopses that these patches fix (that I'm
> aware of). Both were found while I was familiarizing myself with the
> code looking for memory corruption (which turned out to be [0]).
> Either issue would be very difficult to hit in the wild and
> mutex_destroy is empty unless you compile using CONFIG_DEBUG_MUTEXES.

See Documentation/SubmittingPatches which explains what "Fixes:" means.

That same file also contains info in section 2) how to write commit
messages.

Please read it, it'll help you significantly if you're planning on
sending more patches to LKML.

> [0] https://lkml.org/lkml/2016/3/7/1087

Oh, and also, please don't use any web pages archiving lkml as pointers
but the kernel.org redirector:

http://lkml.kernel.org/r/<Message-ID>

or

http://lkml.kernel.org/g/<Message-ID>

(that second one redirects to gmane)

HTH.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 2/2] iommu/amd: Destroy api_lock mutex when freeing domain

[Vesely, Jan]

On Wed, 2016-06-15 at 22:53 +0200, Borislav Petkov wrote:
> On Wed, Jun 15, 2016 at 05:53:15PM +0000, Vesely, Jan wrote:
> > There are no specific bugs/oopses that these patches fix (that I'm
> > aware of). Both were found while I was familiarizing myself with
> > the
> > code looking for memory corruption (which turned out to be [0]).
> > Either issue would be very difficult to hit in the wild and
> > mutex_destroy is empty unless you compile using
> > CONFIG_DEBUG_MUTEXES.
> 
> See Documentation/SubmittingPatches which explains what "Fixes:"
> means.
> 
> That same file also contains info in section 2) how to write commit
> messages.
> 
> Please read it, it'll help you significantly if you're planning on
> sending more patches to LKML.

Hi,

thanks for all the hints. I'll try to revisit and update the patches
soon (might take few weeks though). Kernel development is not my
(current) job and other subsystems don't have these requirements, if
submissions need more than just git send-email of existing fixes
dedicating extra time is difficult.

Jan

> 
> > [0] https://lkml.org/lkml/2016/3/7/1087
> 
> Oh, and also, please don't use any web pages archiving lkml as
> pointers
> but the kernel.org redirector:
> 
> http://lkml.kernel.org/r/<Message-ID>;
> 
> or
> 
> http://lkml.kernel.org/g/<Message-ID>;
> 
> (that second one redirects to gmane)
> 
> HTH.
>