Prev: Fix SVM longmode guests
Next: [PATCH] i2c-imx: do not allow interruptions when waiting for I2C to complete
From: Roedel, Joerg on 27 May 2010 07:10
On Wed, May 26, 2010 at 11:55:59AM -0400, Julia Lawall wrote:
> From: Julia Lawall <julia(a)diku.dk>
>
> Add a spin_unlock missing on the error path. The locks and unlocks are
> balanced in other functions, so it seems that the same should be the case
> here.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)

Applied the attached version of the patch. Thanks for catching this.

Joerg

From 84fe6c19e4a598e8071e3bd1b2c923454eae1268 Mon Sep 17 00:00:00 2001
From: Julia Lawall <julia(a)diku.dk>
Date: Thu, 27 May 2010 12:31:51 +0200
Subject: [PATCH] arch/x86/kernel: Add missing spin_unlock

Add a spin_unlock missing on the error path. The locks and unlocks are
balanced in other functions, so it seems that the same should be the case
here.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E1;
@@

* spin_lock(E1,...);
<+... when != E1
if (...) {
... when != E1
* return ...;
}
...+>
* spin_unlock(E1,...);
// </smpl>

Cc: stable(a)kernel.org
Signed-off-by: Julia Lawall <julia(a)diku.dk>
Signed-off-by: Joerg Roedel <joerg.roedel(a)amd.com>
---
arch/x86/kernel/amd_iommu.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index fa5a147..8a9aaa8 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1487,6 +1487,7 @@ static int __attach_device(struct device *dev,
struct protection_domain *domain)
{
struct iommu_dev_data *dev_data, *alias_data;
+ int ret;

dev_data = get_dev_data(dev);
alias_data = get_dev_data(dev_data->alias);
@@ -1498,13 +1499,14 @@ static int __attach_device(struct device *dev,
spin_lock(&domain->lock);

/* Some sanity checks */
+ ret = -EBUSY;
if (alias_data->domain != NULL &&
alias_data->domain != domain)
- return -EBUSY;
+ goto out_unlock;

if (dev_data->domain != NULL &&
dev_data->domain != domain)
- return -EBUSY;
+ goto out_unlock;

/* Do real assignment */
if (dev_data->alias != dev) {
@@ -1520,10 +1522,14 @@ static int __attach_device(struct device *dev,

atomic_inc(&dev_data->bind);

+ ret = 0;
+
+out_unlock:
+
/* ready */
spin_unlock(&domain->lock);

- return 0;
+ return ret;
}

/*
--
1.7.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Julia Lawall on 27 May 2010 07:20
On Thu, 27 May 2010, Roedel, Joerg wrote:

> On Wed, May 26, 2010 at 11:55:59AM -0400, Julia Lawall wrote:
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression E1;
> > @@
> >
> > * spin_lock(E1,...);
> > <+... when != E1
> > if (...) {
> > ... when != E1
> > * return ...;
> > }
> > ...+>
> > * spin_unlock(E1,...);
> > // </smpl>
>
> Btw, I think it would be great to have a collection of these semantic
> match scripts in the kernel repository together with a build target to
> run these scripts over the kernel sources (like the cscope target).
> Opinions?

We have submitted and received some feedback on an initial version of
this, but I'm not completely sure of the current status.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Roedel, Joerg on 27 May 2010 07:20
On Wed, May 26, 2010 at 11:55:59AM -0400, Julia Lawall wrote:
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression E1;
> @@
>
> * spin_lock(E1,...);
> <+... when != E1
> if (...) {
> ... when != E1
> * return ...;
> }
> ...+>
> * spin_unlock(E1,...);
> // </smpl>

Btw, I think it would be great to have a collection of these semantic
match scripts in the kernel repository together with a build target to
run these scripts over the kernel sources (like the cscope target).
Opinions?

Joerg


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nicolas Palix on 27 May 2010 07:50
On Thursday 27 May 2010 13:17:58 Julia Lawall wrote:
> On Thu, 27 May 2010, Roedel, Joerg wrote:
>
> > On Wed, May 26, 2010 at 11:55:59AM -0400, Julia Lawall wrote:
> > > The semantic match that finds this problem is as follows:
> > > (http://coccinelle.lip6.fr/)
> > >
> > > // <smpl>
> > > @@
> > > expression E1;
> > > @@
> > >
> > > * spin_lock(E1,...);
> > > <+... when != E1
> > > if (...) {
> > > ... when != E1
> > > * return ...;
> > > }
> > > ...+>
> > > * spin_unlock(E1,...);
> > > // </smpl>
> >
> > Btw, I think it would be great to have a collection of these semantic
> > match scripts in the kernel repository together with a build target to
> > run these scripts over the kernel sources (like the cscope target).
> > Opinions?
>
> We have submitted and received some feedback on an initial version of
> this, but I'm not completely sure of the current status.

You can see the latest feedback we get at
http://lkml.org/lkml/2010/5/10/257

The initial submission and its comments are at
http://lkml.org/lkml/2010/4/26/269

>
> julia
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

--
Nicolas Palix
Tel: (+33) 1 44 27 87 25
Tel: (+33) 6 81 07 91 72
Web: http://www.diku.dk/~npalix/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Roedel, Joerg on 28 May 2010 03:20
On Thu, May 27, 2010 at 07:42:25AM -0400, Nicolas Palix wrote:
> > We have submitted and received some feedback on an initial version of
> > this, but I'm not completely sure of the current status.
>
> You can see the latest feedback we get at
> http://lkml.org/lkml/2010/5/10/257
>
> The initial submission and its comments are at
> http://lkml.org/lkml/2010/4/26/269

I've also sent some feedback. Would be cool if you could work the
feedback in and do a repost asking Andrew to take it. Would be cool to
have this merged with 2.6.36.

Joerg


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3
Prev: Fix SVM longmode guests
Next: [PATCH] i2c-imx: do not allow interruptions when waiting for I2C to complete