diff mbox

[Karmic] SRU: Upstream detected regression in 2.6.31.7 (re-send)

Message ID 1264158953-29365-1-git-send-email-stefan.bader@canonical.com
State Accepted
Delegated to: Stefan Bader
Headers show

Commit Message

Stefan Bader Jan. 22, 2010, 11:15 a.m. UTC 
Resending the patch as it now has hit both upstream and 2.6.32.y.
Unfortunately 2.6.31.y support ended by now.

-Stefan

From 8dfabfcf7b0c03f33f7cb945772027060de70b61 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sat, 19 Dec 2009 14:43:01 +0000
Subject: [PATCH] modules: Skip empty sections when exporting section notes

BugLink: http://bugs.launchpad.net/bugs/511129

commit 10b465aaf9536ee5a16652fa0700740183d48ec9 upstream.

Commit 35dead4 "modules: don't export section names of empty sections
via sysfs" changed the set of sections that have attributes, but did
not change the iteration over these attributes in add_notes_attrs().
This can lead to add_notes_attrs() creating attributes with the wrong
names or with null name pointers.

Introduce a sect_empty() function and use it in both add_sect_attrs()
and add_notes_attrs().

Reported-by: Martin Michlmayr <tbm@cyrius.com>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Tested-by: Martin Michlmayr <tbm@cyrius.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>
---
 kernel/module.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

Comments

Colin Ian King Jan. 25, 2010, 11:35 a.m. UTC | #1 
On Fri, 2010-01-22 at 12:15 +0100, Stefan Bader wrote:
> Resending the patch as it now has hit both upstream and 2.6.32.y.
> Unfortunately 2.6.31.y support ended by now.
> 
> -Stefan
> 
> From 8dfabfcf7b0c03f33f7cb945772027060de70b61 Mon Sep 17 00:00:00 2001
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sat, 19 Dec 2009 14:43:01 +0000
> Subject: [PATCH] modules: Skip empty sections when exporting section notes
> 
> BugLink: http://bugs.launchpad.net/bugs/511129
> 
> commit 10b465aaf9536ee5a16652fa0700740183d48ec9 upstream.
> 
> Commit 35dead4 "modules: don't export section names of empty sections
> via sysfs" changed the set of sections that have attributes, but did
> not change the iteration over these attributes in add_notes_attrs().
> This can lead to add_notes_attrs() creating attributes with the wrong
> names or with null name pointers.
> 
> Introduce a sect_empty() function and use it in both add_sect_attrs()
> and add_notes_attrs().
> 
> Reported-by: Martin Michlmayr <tbm@cyrius.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Tested-by: Martin Michlmayr <tbm@cyrius.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>
> ---
>  kernel/module.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 5842a71..37caef0 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1146,6 +1146,12 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs,
>   * J. Corbet <corbet@lwn.net>
>   */
>  #if defined(CONFIG_KALLSYMS) && defined(CONFIG_SYSFS)
> +
> +static inline bool sect_empty(const Elf_Shdr *sect)
> +{
> +	return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
> +}
> +
>  struct module_sect_attr
>  {
>  	struct module_attribute mattr;
> @@ -1187,8 +1193,7 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
>  
>  	/* Count loaded sections and allocate structures */
>  	for (i = 0; i < nsect; i++)
> -		if (sechdrs[i].sh_flags & SHF_ALLOC
> -		    && sechdrs[i].sh_size)
> +		if (!sect_empty(&sechdrs[i]))
>  			nloaded++;
>  	size[0] = ALIGN(sizeof(*sect_attrs)
>  			+ nloaded * sizeof(sect_attrs->attrs[0]),
> @@ -1206,9 +1211,7 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
>  	sattr = &sect_attrs->attrs[0];
>  	gattr = &sect_attrs->grp.attrs[0];
>  	for (i = 0; i < nsect; i++) {
> -		if (! (sechdrs[i].sh_flags & SHF_ALLOC))
> -			continue;
> -		if (!sechdrs[i].sh_size)
> +		if (sect_empty(&sechdrs[i]))
>  			continue;
>  		sattr->address = sechdrs[i].sh_addr;
>  		sattr->name = kstrdup(secstrings + sechdrs[i].sh_name,
> @@ -1292,7 +1295,7 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect,
>  	/* Count notes sections and allocate structures.  */
>  	notes = 0;
>  	for (i = 0; i < nsect; i++)
> -		if ((sechdrs[i].sh_flags & SHF_ALLOC) &&
> +		if (!sect_empty(&sechdrs[i]) &&
>  		    (sechdrs[i].sh_type == SHT_NOTE))
>  			++notes;
>  
> @@ -1308,7 +1311,7 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect,
>  	notes_attrs->notes = notes;
>  	nattr = &notes_attrs->attrs[0];
>  	for (loaded = i = 0; i < nsect; ++i) {
> -		if (!(sechdrs[i].sh_flags & SHF_ALLOC))
> +		if (sect_empty(&sechdrs[i]))
>  			continue;
>  		if (sechdrs[i].sh_type == SHT_NOTE) {
>  			nattr->attr.name = mod->sect_attrs->attrs[loaded].name;
> -- 
> 1.6.3.3
> 
> 
Looks reasonable to me and sect_empty() makes the code clearer too.

Acked-by: Colin King <colin.king@canonical.com>
Tim Gardner Jan. 25, 2010, 2:08 p.m. UTC | #2 
Stefan Bader wrote:
> Resending the patch as it now has hit both upstream and 2.6.32.y.
> Unfortunately 2.6.31.y support ended by now.
> 
> -Stefan
> 
> From 8dfabfcf7b0c03f33f7cb945772027060de70b61 Mon Sep 17 00:00:00 2001
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sat, 19 Dec 2009 14:43:01 +0000
> Subject: [PATCH] modules: Skip empty sections when exporting section notes
> 
> BugLink: http://bugs.launchpad.net/bugs/511129
> 
> commit 10b465aaf9536ee5a16652fa0700740183d48ec9 upstream.
> 
> Commit 35dead4 "modules: don't export section names of empty sections
> via sysfs" changed the set of sections that have attributes, but did
> not change the iteration over these attributes in add_notes_attrs().
> This can lead to add_notes_attrs() creating attributes with the wrong
> names or with null name pointers.
> 
> Introduce a sect_empty() function and use it in both add_sect_attrs()
> and add_notes_attrs().
> 
> Reported-by: Martin Michlmayr <tbm@cyrius.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Tested-by: Martin Michlmayr <tbm@cyrius.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>
> ---
>  kernel/module.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 5842a71..37caef0 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1146,6 +1146,12 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs,
>   * J. Corbet <corbet@lwn.net>
>   */
>  #if defined(CONFIG_KALLSYMS) && defined(CONFIG_SYSFS)
> +
> +static inline bool sect_empty(const Elf_Shdr *sect)
> +{
> +	return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
> +}
> +
>  struct module_sect_attr
>  {
>  	struct module_attribute mattr;
> @@ -1187,8 +1193,7 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
>  
>  	/* Count loaded sections and allocate structures */
>  	for (i = 0; i < nsect; i++)
> -		if (sechdrs[i].sh_flags & SHF_ALLOC
> -		    && sechdrs[i].sh_size)
> +		if (!sect_empty(&sechdrs[i]))
>  			nloaded++;
>  	size[0] = ALIGN(sizeof(*sect_attrs)
>  			+ nloaded * sizeof(sect_attrs->attrs[0]),
> @@ -1206,9 +1211,7 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect,
>  	sattr = &sect_attrs->attrs[0];
>  	gattr = &sect_attrs->grp.attrs[0];
>  	for (i = 0; i < nsect; i++) {
> -		if (! (sechdrs[i].sh_flags & SHF_ALLOC))
> -			continue;
> -		if (!sechdrs[i].sh_size)
> +		if (sect_empty(&sechdrs[i]))
>  			continue;
>  		sattr->address = sechdrs[i].sh_addr;
>  		sattr->name = kstrdup(secstrings + sechdrs[i].sh_name,
> @@ -1292,7 +1295,7 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect,
>  	/* Count notes sections and allocate structures.  */
>  	notes = 0;
>  	for (i = 0; i < nsect; i++)
> -		if ((sechdrs[i].sh_flags & SHF_ALLOC) &&
> +		if (!sect_empty(&sechdrs[i]) &&
>  		    (sechdrs[i].sh_type == SHT_NOTE))
>  			++notes;
>  
> @@ -1308,7 +1311,7 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect,
>  	notes_attrs->notes = notes;
>  	nattr = &notes_attrs->attrs[0];
>  	for (loaded = i = 0; i < nsect; ++i) {
> -		if (!(sechdrs[i].sh_flags & SHF_ALLOC))
> +		if (sect_empty(&sechdrs[i]))
>  			continue;
>  		if (sechdrs[i].sh_type == SHT_NOTE) {
>  			nattr->attr.name = mod->sect_attrs->attrs[loaded].name;

Acked-by: Tim Gardner <tim.gardner@canonical.com>
Stefan Bader Jan. 25, 2010, 7:34 p.m. UTC | #3 
Applied to Karmic, thanks.
diff mbox

Patch

diff --git a/kernel/module.c b/kernel/module.c
index 5842a71..37caef0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1146,6 +1146,12 @@  static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs,
  * J. Corbet <corbet@lwn.net>
  */
 #if defined(CONFIG_KALLSYMS) && defined(CONFIG_SYSFS)
+
+static inline bool sect_empty(const Elf_Shdr *sect)
+{
+	return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
+}
+
 struct module_sect_attr
 {
 	struct module_attribute mattr;
@@ -1187,8 +1193,7 @@  static void add_sect_attrs(struct module *mod, unsigned int nsect,
 
 	/* Count loaded sections and allocate structures */
 	for (i = 0; i < nsect; i++)
-		if (sechdrs[i].sh_flags & SHF_ALLOC
-		    && sechdrs[i].sh_size)
+		if (!sect_empty(&sechdrs[i]))
 			nloaded++;
 	size[0] = ALIGN(sizeof(*sect_attrs)
 			+ nloaded * sizeof(sect_attrs->attrs[0]),
@@ -1206,9 +1211,7 @@  static void add_sect_attrs(struct module *mod, unsigned int nsect,
 	sattr = &sect_attrs->attrs[0];
 	gattr = &sect_attrs->grp.attrs[0];
 	for (i = 0; i < nsect; i++) {
-		if (! (sechdrs[i].sh_flags & SHF_ALLOC))
-			continue;
-		if (!sechdrs[i].sh_size)
+		if (sect_empty(&sechdrs[i]))
 			continue;
 		sattr->address = sechdrs[i].sh_addr;
 		sattr->name = kstrdup(secstrings + sechdrs[i].sh_name,
@@ -1292,7 +1295,7 @@  static void add_notes_attrs(struct module *mod, unsigned int nsect,
 	/* Count notes sections and allocate structures.  */
 	notes = 0;
 	for (i = 0; i < nsect; i++)
-		if ((sechdrs[i].sh_flags & SHF_ALLOC) &&
+		if (!sect_empty(&sechdrs[i]) &&
 		    (sechdrs[i].sh_type == SHT_NOTE))
 			++notes;
 
@@ -1308,7 +1311,7 @@  static void add_notes_attrs(struct module *mod, unsigned int nsect,
 	notes_attrs->notes = notes;
 	nattr = &notes_attrs->attrs[0];
 	for (loaded = i = 0; i < nsect; ++i) {
-		if (!(sechdrs[i].sh_flags & SHF_ALLOC))
+		if (sect_empty(&sechdrs[i]))
 			continue;
 		if (sechdrs[i].sh_type == SHT_NOTE) {
 			nattr->attr.name = mod->sect_attrs->attrs[loaded].name;