Re: regression: crash from 'ls /sys/modules/wl1251_spi/notes'

[Helge Deller]

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...

Helge


> ---
>
> diff --git a/kernel/module.c b/kernel/module.c
> index e96b8ed..957f912 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -132,6 +132,11 @@ void __module_put_and_exit(struct module *mod, long code)
>   }
>   EXPORT_SYMBOL(__module_put_and_exit);
>
> +static inline int section_allocated(Elf_Shdr hdr)
> +{
> +	return (hdr.sh_flags&  SHF_ALLOC)&&  hdr.sh_size != 0;
> +}
> +
>   /* Find a module section: 0 means not found. */
>   static unsigned int find_sec(Elf_Ehdr *hdr,
>   			     Elf_Shdr *sechdrs,
> @@ -142,7 +147,7 @@ static unsigned int find_sec(Elf_Ehdr *hdr,
>
>   	for (i = 1; i<  hdr->e_shnum; i++)
>   		/* Alloc bit cleared means "ignore it." */
> -		if ((sechdrs[i].sh_flags&  SHF_ALLOC)
> +		if (section_allocated(sechdrs[i])
>   		&&  strcmp(secstrings+sechdrs[i].sh_name, name) == 0)
>   			return i;
>   	return 0;
> @@ -1051,8 +1056,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 (section_allocated(sechdrs[i]))
>   			nloaded++;
>   	size[0] = ALIGN(sizeof(*sect_attrs)
>   			+ nloaded * sizeof(sect_attrs->attrs[0]),
> @@ -1070,9 +1074,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 (!section_allocated(sechdrs[i]))
>   			continue;
>   		sattr->address = sechdrs[i].sh_addr;
>   		sattr->name = kstrdup(secstrings + sechdrs[i].sh_name,
> @@ -1156,7 +1158,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 (section_allocated(sechdrs[i])&&
>   		(sechdrs[i].sh_type == SHT_NOTE))
>   			++notes;
>
> @@ -1172,7 +1174,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 (!section_allocated(sechdrs[i]))
>   			continue;
>   		if (sechdrs[i].sh_type == SHT_NOTE) {
>   			nattr->attr.name = mod->sect_attrs->attrs[loaded].name;
> @@ -1720,7 +1722,7 @@ static char elf_type(const Elf_Sym *sym,
>   		return '?';
>   	if (sechdrs[sym->st_shndx].sh_flags&  SHF_EXECINSTR)
>   		return 't';
> -	if (sechdrs[sym->st_shndx].sh_flags&  SHF_ALLOC
> +	if (section_allocated(sechdrs[sym->st_shndx])
>   	&&  sechdrs[sym->st_shndx].sh_type != SHT_NOBITS) {
>   		if (!(sechdrs[sym->st_shndx].sh_flags&  SHF_WRITE))
>   			return 'r';
> @@ -1751,7 +1753,7 @@ static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
>   		return false;
>
>   	sec = sechdrs + src->st_shndx;
> -	if (!(sec->sh_flags&  SHF_ALLOC)
> +	if (!section_allocated(*sec)
>   #ifndef CONFIG_KALLSYMS_ALL
>   	    || !(sec->sh_flags&  SHF_EXECINSTR)
>   #endif
> @@ -1913,7 +1915,7 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
>   	kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL);
>
>   	for (i = 1; i<  hdr->e_shnum; i++) {
> -		if (!(sechdrs[i].sh_flags&  SHF_ALLOC))
> +		if (!section_allocated(sechdrs[i]))
>   			continue;
>   		if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0
>   		&&  strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0)
> @@ -2139,7 +2141,7 @@ static noinline struct module *load_module(void __user *umod,
>   	for (i = 0; i<  hdr->e_shnum; i++) {
>   		void *dest;
>
> -		if (!(sechdrs[i].sh_flags&  SHF_ALLOC))
> +		if (!section_allocated(sechdrs[i]))
>   			continue;
>
>   		if (sechdrs[i].sh_entsize&  INIT_OFFSET_MASK)
> @@ -2287,7 +2289,7 @@ static noinline struct module *load_module(void __user *umod,
>   			continue;
>
>   		/* Don't bother with non-allocated sections */
> -		if (!(sechdrs[info].sh_flags&  SHF_ALLOC))
> +		if (!section_allocated(sechdrs[info]))
>   			continue;
>
>   		if (sechdrs[i].sh_type == SHT_REL)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/