[PATCH 0/4] iommu: Prevent oops in iommu_get() and while arch_iommu is in use

[PATCH 0/4] iommu: Prevent oops in iommu_get() and while arch_iommu is in use

[Sakari Ailus]

Hi,

[Resend: the patches were accidentally sent to linux-media instead.
Apologies.]

This patchset is aimed to fix a problem in arch_iommu implementation
references. When an actual arch_iommu implementation is not loaded while
iommu_get() is being called results to a kernel oops, as well as
removing an arch_iommu implementation which is in use.

This patchset fixes both issues.

The patchset assumes the arch_iommu is uninstalled at module unload
time. Is this an acceptable requirement?

Serialisation of the access to arch_iommu is done using mutex called
arch_iommu_mutex.

module_put() doesn't need to have the arch_iommu_mutex since when this
gets called there won't be any users on the arch_iommu anyway.

Comments are welcome. :-)

Cheers,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

[PATCH 1/4] omap iommu: Check existence of arch_iommu

[Sakari Ailus]

Check that the arch_iommu has been installed before trying to use it. This
will lead to kernel oops if the arch_iommu isn't there.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 arch/arm/plat-omap/iommu.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index b1107c0..f0fea0b 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -104,6 +104,9 @@ static int iommu_enable(struct iommu *obj)
 	if (!obj)
 		return -EINVAL;
 
+	if (!arch_iommu)
+		return -ENOENT;
+
 	clk_enable(obj->clk);
 
 	err = arch_iommu->enable(obj);
-- 
1.7.2.3

[PATCH 2/4] omap iommu: Add module information to struct iommu_functions

[Sakari Ailus]

Whichever module that implements the struct, may not be unloaded while it's
in use.  Prepare to this by adding module reference to the structure.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 arch/arm/plat-omap/include/plat/iommu.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index 69230d6..26fefb4 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -79,6 +79,7 @@ struct iotlb_lock {
 /* architecture specific functions */
 struct iommu_functions {
 	unsigned long	version;
+	struct module *module;
 
 	int (*enable)(struct iommu *obj);
 	void (*disable)(struct iommu *obj);
-- 
1.7.2.3

[PATCH 3/4] omap2 iommu: Set module information in omap2_iommu_ops

[Sakari Ailus]

Set omap2_iommu_ops.module to point to THIS_MODULE.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 arch/arm/mach-omap2/iommu2.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..cf0c32a 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -342,6 +342,8 @@ static const struct iommu_functions omap2_iommu_ops = {
 	.save_ctx	= omap2_iommu_save_ctx,
 	.restore_ctx	= omap2_iommu_restore_ctx,
 	.dump_ctx	= omap2_iommu_dump_ctx,
+
+	.module		= THIS_MODULE,
 };
 
 static int __init omap2_iommu_init(void)
-- 
1.7.2.3

[PATCH 4/4] omap iommu: Prevent iommu implementations from being unloaded while in use

[Sakari Ailus]

Prevent module implementing arch_iommu from being unloaded while the
arch_iommu is in use, essentially while the iommu has been acquired using
iommu_get() by increasing module use count.

This assumes that the arch_iommu has to be uninstalled at module unload
time.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 arch/arm/plat-omap/iommu.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index f0fea0b..430ed94 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -35,6 +35,7 @@ static const struct iommu_functions *arch_iommu;
 
 static struct platform_driver omap_iommu_driver;
 static struct kmem_cache *iopte_cachep;
+static struct mutex arch_iommu_mutex;
 
 /**
  * install_iommu_arch - Install archtecure specific iommu functions
@@ -45,10 +46,17 @@ static struct kmem_cache *iopte_cachep;
  **/
 int install_iommu_arch(const struct iommu_functions *ops)
 {
-	if (arch_iommu)
+	mutex_lock(&arch_iommu_mutex);
+
+	if (arch_iommu) {
+		mutex_unlock(&arch_iommu_mutex);
 		return -EBUSY;
+	}
 
 	arch_iommu = ops;
+
+	mutex_unlock(&arch_iommu_mutex);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(install_iommu_arch);
@@ -58,13 +66,22 @@ EXPORT_SYMBOL_GPL(install_iommu_arch);
  * @ops:	a pointer to architecture specific iommu functions
  *
  * This interface uninstalls the iommu algorighm installed previously.
+ *
+ * Note: This function may only be called at module_exit() function of
+ * a module which implements arch_iommu. Otherwise another mechanism
+ * from the module use count only to ensure the arch_iommu stays there
+ * has to be implemented.
  **/
 void uninstall_iommu_arch(const struct iommu_functions *ops)
 {
+	mutex_lock(&arch_iommu_mutex);
+
 	if (arch_iommu != ops)
 		pr_err("%s: not your arch\n", __func__);
 
 	arch_iommu = NULL;
+
+	mutex_unlock(&arch_iommu_mutex);
 }
 EXPORT_SYMBOL_GPL(uninstall_iommu_arch);
 
@@ -104,14 +121,20 @@ static int iommu_enable(struct iommu *obj)
 	if (!obj)
 		return -EINVAL;
 
-	if (!arch_iommu)
+	mutex_lock(&arch_iommu_mutex);
+	if (!arch_iommu || !try_module_get(arch_iommu->module)) {
+		mutex_unlock(&arch_iommu_mutex);
 		return -ENOENT;
+	}
 
 	clk_enable(obj->clk);
 
 	err = arch_iommu->enable(obj);
 
 	clk_disable(obj->clk);
+
+	mutex_unlock(&arch_iommu_mutex);
+
 	return err;
 }
 
@@ -125,6 +148,8 @@ static void iommu_disable(struct iommu *obj)
 	arch_iommu->disable(obj);
 
 	clk_disable(obj->clk);
+
+	module_put(arch_iommu->module);
 }
 
 /*
@@ -1058,6 +1083,8 @@ static int __init omap_iommu_init(void)
 		return -ENOMEM;
 	iopte_cachep = p;
 
+	mutex_init(&arch_iommu_mutex);
+
 	return platform_driver_register(&omap_iommu_driver);
 }
 module_init(omap_iommu_init);
-- 
1.7.2.3

Re: [PATCH 0/4] iommu: Prevent oops in iommu_get() and while arch_iommu is in use

[David Cohen]

On Fri, Mar 25, 2011 at 5:17 PM, Sakari Ailus
<sakari.ailus@maxwell.research.nokia.com> wrote:
> Hi,

Hi Sakari,

>
> [Resend: the patches were accidentally sent to linux-media instead.
> Apologies.]
>
> This patchset is aimed to fix a problem in arch_iommu implementation
> references. When an actual arch_iommu implementation is not loaded while
> iommu_get() is being called results to a kernel oops, as well as
> removing an arch_iommu implementation which is in use.
>
> This patchset fixes both issues.

Sounds nice.

>
> The patchset assumes the arch_iommu is uninstalled at module unload
> time. Is this an acceptable requirement?

I can't see a reason why this assumption could be wrong. In my point
of view it's acceptable. Let's see Hiroshi's.

Regards,

David Cohen

>
> Serialisation of the access to arch_iommu is done using mutex called
> arch_iommu_mutex.
>
> module_put() doesn't need to have the arch_iommu_mutex since when this
> gets called there won't be any users on the arch_iommu anyway.
>
> Comments are welcome. :-)
>
> Cheers,
>
> --
> Sakari Ailus
> sakari.ailus@maxwell.research.nokia.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html