regression: crash from 'ls /sys/modules/wl1251_spi/notes'
in [Kernel]
|
Prev: [PATCH] some clean up in fs/proc
Next: [tip:x86/asm] x86-64: Modify copy_user_generic() alternatives mechanism
From: James Bottomley on 2 Jan 2010 11:40 On Fri, 2010-01-01 at 11:08 +0800, Américo Wang wrote: > This patch looks fine for me, except that I don't think it's necessary > to introduce an inline function for that... Actually, I really think we do. The whole reason we got into this mess in the first place is that it wasn't obvious from the code that if we altered the loadability of sections, there were a ton of dependent places we also had to update. Putting all that knowledge into a single inline makes the same mistake impossible to make because there's now only one place to go to adjust the loadability of sections, and it automatically takes care of all the dependencies. > Reviewed-by: WANG Cong <xiyou.wangcong(a)gmail.com> Thanks, James -- 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: Roland McGrath on 4 Jan 2010 13:30 Acked-by: Roland McGrath <roland(a)redhat.com> -- 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: Andrew Morton on 5 Jan 2010 20:20
On Thu, 31 Dec 2009 22:15:08 +0100 Helge Deller <deller(a)gmx.de> wrote: > On 12/30/2009 04:49 PM, James Bottomley wrote: > > A better, and more comprehensive patch would be to try not to count the > > empty text sections when we're building the notes section (and actually > > anywhere else in the file). This patch actually relies on the fact that > > if sh_size is zero for the text section it should be for the > > corresponding notes section. If that doesn't work, we'd actually have > > to do the matching in the construction piece. > > > > Can you try it to see if it works for you? If it doesn't, I'll try > > matching notes to text. It works fine on parisc, but as we don't have a > > notes section, that's not saying much ... > > > > Thanks, > > > > James > > > Ben Hutchings already sent a similar patch. > See: http://patchwork.kernel.org/patch/68925/ > > IMHO James patch below seems better since it > checks if a section will be allocated at a few more > places... > Ben's patch (which is below) is currently in linux-next, via a Rusty tree. It is marked for -stable backporting. If James's patch is preferable then there's an opportunity to do the swap if we move promptly. commit 9e9b48a89ed43c73d7355ff999b8e87b0628e1cd Author: Ben Hutchings <ben(a)decadent.org.uk> AuthorDate: Sat Dec 19 14:43:01 2009 +0000 Commit: Stephen Rothwell <sfr(a)canb.auug.org.au> CommitDate: Tue Jan 5 08:44:50 2010 +1100 modules: Skip empty sections when exporting section notes 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(a)cyrius.com> Signed-off-by: Ben Hutchings <ben(a)decadent.org.uk> Tested-by: Martin Michlmayr <tbm(a)cyrius.com> Cc: stable(a)kernel.org Signed-off-by: Rusty Russell <rusty(a)rustcorp.com.au> diff --git a/kernel/module.c b/kernel/module.c index e96b8ed..f82386b 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1010,6 +1010,12 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs, * J. Corbet <corbet(a)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; @@ -1051,8 +1057,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]), @@ -1070,9 +1075,7 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect, sattr = §_attrs->attrs[0]; gattr = §_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, @@ -1156,7 +1159,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; @@ -1172,7 +1175,7 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect, notes_attrs->notes = notes; nattr = ¬es_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; -- 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/ |