Hi Russell,
On Monday 03 October 2011 03:29 PM, Russell King - ARM Linux wrote:
On Mon, Oct 03, 2011 at 03:10:41PM +0530, Tushar Behera wrote:
Some of the functions and structures did not have _init or __initdata attributes, even though they were referenced from functions / structures with those attribute, resulting in section mismatches.
Firstly - it's a good idea to include the warnings which you're fixing in the commit log text, so that people know exactly what is being fixed.
Thanks for your review.
Sure, I will add it in next revision.
[ snip ]
diff --git a/arch/arm/mach-s3c2416/irq.c b/arch/arm/mach-s3c2416/irq.c index 28ad20d..153cb2f 100644 --- a/arch/arm/mach-s3c2416/irq.c +++ b/arch/arm/mach-s3c2416/irq.c @@ -234,7 +234,7 @@ static int __init s3c2416_irq_add(struct sys_device *sysdev) return 0; }
-static struct sysdev_driver s3c2416_irq_driver = { +static struct sysdev_driver s3c2416_irq_driver __initdata = { .add = s3c2416_irq_add, };
I remain entirely unconvinced that this is correct. As a result of the "sysdev_driver_register(&s3c2416_sysclass,&s3c2416_irq_driver);" call, this structure is placed on a list.
If this structure is marked __initdata, then the memory behind the structure will be freed and overwritten - however, it's still on a list which might be walked. Such a walk would cause a kernel oops or might even be an exploitable security hole if that page ends up in userspace - especially as said structure contains function calls which would be called in privileged mode.
The function s3c2416_irq_add() is defined with __init attribute. Also a cascade of functions called from s3c2416_irq_add() are also defined with __init attribute.
Would it be a good idea to remove __init attribute of all these functions (there are 2 of them) called from s3c2416_irq_add() instead?
The same comment applies to the other sysdev driver structures you're marking __initdata too.