[KJ] [KJ PATCH] sh: fix proc file removal for superh store queue

[KJ] [KJ PATCH] sh: fix proc file removal for superh store queue

[Neil Horman]

Patch to balance proc file creation and removal for superh store queue module.
Currently a failure in module load, or a module unload leaves a proc file
registered, causing an oops in the event the proc file is read.  This patch
corrects that.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 sq.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)


diff --git a/arch/sh/kernel/cpu/sh4/sq.c b/arch/sh/kernel/cpu/sh4/sq.c
index 781dbb1..18b092c 100644
--- a/arch/sh/kernel/cpu/sh4/sq.c
+++ b/arch/sh/kernel/cpu/sh4/sq.c
@@ -421,18 +421,27 @@ static struct miscdevice sq_dev = {
 
 static int __init sq_api_init(void)
 {
+	int ret;
 	printk(KERN_NOTICE "sq: Registering store queue API.\n");
 
 #ifdef CONFIG_PROC_FS
 	create_proc_read_entry("sq_mapping", 0, 0, sq_mapping_read_proc, 0);
 #endif
 
-	return misc_register(&sq_dev);
+	ret = misc_register(&sq_dev);
+#ifdef CONFIG_PROC_FS
+	if (ret) 
+		remove_proc_entry("sq_mapping", NULL);
+#endif
+	return ret;
 }
 
 static void __exit sq_api_exit(void)
 {
 	misc_deregister(&sq_dev);
+#ifdef CONFIG_PROC_FS
+	remove_proc_entry("sq_mapping", NULL);
+#endif
 }
 
 module_init(sq_api_init);
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [KJ PATCH] sh: fix proc file removal for superh store

[Andrew Morton]

> To: kernel-janitors@lists.osdl.org

An oops fix isn't a kernel-janitors thing.  It's a
slam-it-into-mainline-and-consider-a-backport thing.

On Thu, 3 Aug 2006 15:18:28 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> Patch to balance proc file creation and removal for superh store queue module.

You have a superh machine?

> Currently a failure in module load, or a module unload leaves a proc file
> registered, causing an oops in the event the proc file is read.  This patch
> corrects that.
> 
> Thanks & Regards
> Neil
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 
>  sq.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/arch/sh/kernel/cpu/sh4/sq.c b/arch/sh/kernel/cpu/sh4/sq.c
> index 781dbb1..18b092c 100644
> --- a/arch/sh/kernel/cpu/sh4/sq.c
> +++ b/arch/sh/kernel/cpu/sh4/sq.c
> @@ -421,18 +421,27 @@ static struct miscdevice sq_dev = {
>  
>  static int __init sq_api_init(void)
>  {
> +	int ret;
>  	printk(KERN_NOTICE "sq: Registering store queue API.\n");
>  
>  #ifdef CONFIG_PROC_FS
>  	create_proc_read_entry("sq_mapping", 0, 0, sq_mapping_read_proc, 0);
>  #endif
>  
> -	return misc_register(&sq_dev);
> +	ret = misc_register(&sq_dev);
> +#ifdef CONFIG_PROC_FS
> +	if (ret) 
> +		remove_proc_entry("sq_mapping", NULL);
> +#endif
> +	return ret;
>  }
>  
>  static void __exit sq_api_exit(void)
>  {
>  	misc_deregister(&sq_dev);
> +#ifdef CONFIG_PROC_FS
> +	remove_proc_entry("sq_mapping", NULL);
> +#endif
>  }
>  
>  module_init(sq_api_init);

OK.  But we do have a stub which provides a CONFIG_PROC_FS=n implementation
of remove_proc_entry(), so the ifdefs shouldn't be necessary?
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [KJ PATCH] sh: fix proc file removal for superh store

[Neil Horman]

On Thu, Aug 03, 2006 at 12:42:35PM -0700, Andrew Morton wrote:
> 
> > To: kernel-janitors@lists.osdl.org
> 
> An oops fix isn't a kernel-janitors thing.  It's a
> slam-it-into-mainline-and-consider-a-backport thing.
> 
sorry, I was looking for more misc_register cleanups for KJ, and ran into this
in the process.  Wrote the email without thinking about the subject tags, my
bad.

> On Thu, 3 Aug 2006 15:18:28 -0400
> Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Patch to balance proc file creation and removal for superh store queue module.
> 
> You have a superh machine?
> 
No, but the problem is fairly self evident.  leaving a proc file dangling on
module removal is begging to execute random data.

> 
> OK.  But we do have a stub which provides a CONFIG_PROC_FS=n implementation
> of remove_proc_entry(), so the ifdefs shouldn't be necessary?
Good point, I'll remove, and repost.
Neil

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] sh: fix proc file removal for superh store queue

[Neil Horman]

Patch to clean up proc file removal in sq module for superh arch.  currently on
a failed module load or on module unload a proc file is left registered which
can cause a random memory execution or oopses if read after unload.  This patch
cleans up that deregistration.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 sq.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)


diff --git a/arch/sh/kernel/cpu/sh4/sq.c b/arch/sh/kernel/cpu/sh4/sq.c
index 781dbb1..4b2b0b1 100644
--- a/arch/sh/kernel/cpu/sh4/sq.c
+++ b/arch/sh/kernel/cpu/sh4/sq.c
@@ -421,18 +421,22 @@ static struct miscdevice sq_dev = {
 
 static int __init sq_api_init(void)
 {
+	int ret;
 	printk(KERN_NOTICE "sq: Registering store queue API.\n");
 
-#ifdef CONFIG_PROC_FS
 	create_proc_read_entry("sq_mapping", 0, 0, sq_mapping_read_proc, 0);
-#endif
 
-	return misc_register(&sq_dev);
+	ret = misc_register(&sq_dev);
+	if (ret) 
+		remove_proc_entry("sq_mapping", NULL);
+
+	return ret;
 }
 
 static void __exit sq_api_exit(void)
 {
 	misc_deregister(&sq_dev);
+	remove_proc_entry("sq_mapping", NULL);
 }
 
 module_init(sq_api_init);
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [PATCH] sh: fix proc file removal for superh store queue module

[Neil Horman]

Patch to clean up proc file removal in sq module for superh arch.  currently on
a failed module load or on module unload a proc file is left registered which
can cause a random memory execution or oopses if read after unload.  This patch
cleans up that deregistration.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 sq.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)


diff --git a/arch/sh/kernel/cpu/sh4/sq.c b/arch/sh/kernel/cpu/sh4/sq.c
index 781dbb1..4b2b0b1 100644
--- a/arch/sh/kernel/cpu/sh4/sq.c
+++ b/arch/sh/kernel/cpu/sh4/sq.c
@@ -421,18 +421,22 @@ static struct miscdevice sq_dev = {
 
 static int __init sq_api_init(void)
 {
+	int ret;
 	printk(KERN_NOTICE "sq: Registering store queue API.\n");
 
-#ifdef CONFIG_PROC_FS
 	create_proc_read_entry("sq_mapping", 0, 0, sq_mapping_read_proc, 0);
-#endif
 
-	return misc_register(&sq_dev);
+	ret = misc_register(&sq_dev);
+	if (ret) 
+		remove_proc_entry("sq_mapping", NULL);
+
+	return ret;
 }
 
 static void __exit sq_api_exit(void)
 {
 	misc_deregister(&sq_dev);
+	remove_proc_entry("sq_mapping", NULL);
 }
 
 module_init(sq_api_init);

Re: [KJ] [linuxsh-dev] [PATCH] sh: fix proc file removal

[Paul Mundt]

On Thu, 2006-08-03 at 16:18 -0400, Neil Horman wrote:
> Patch to clean up proc file removal in sq module for superh arch.  currently on
> a failed module load or on module unload a proc file is left registered which
> can cause a random memory execution or oopses if read after unload.  This patch
> cleans up that deregistration.
> 
Looks good, thanks Neil.

Acked-by: Paul Mundt <paul.mundt@renesas.com>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [linuxsh-dev] [PATCH] sh: fix proc file removal for superh store queue module

[Paul Mundt]

On Thu, 2006-08-03 at 16:18 -0400, Neil Horman wrote:
> Patch to clean up proc file removal in sq module for superh arch.  currently on
> a failed module load or on module unload a proc file is left registered which
> can cause a random memory execution or oopses if read after unload.  This patch
> cleans up that deregistration.
> 
Looks good, thanks Neil.

Acked-by: Paul Mundt <paul.mundt@renesas.com>