From: James Bottomley on
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
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
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 = &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,
@@ -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 = &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;

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