lkml.org 
[lkml]   [2019]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] modpost: Copy namespace string into 'struct symbol'
On Fri, Sep 27, 2019 at 09:03:46AM +0100, Matthias Maennich wrote:
> On Thu, Sep 26, 2019 at 05:24:46PM -0500, Shaun Ruffell wrote:
> > When building an out-of-tree module I was receiving many warnings from
> > modpost like:
> >
> > WARNING: module dahdi_vpmadt032_loader uses symbol __kmalloc from namespace ts/dahdi-linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> > WARNING: module dahdi_vpmadt032_loader uses symbol vpmadtreg_register from namespace linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> > WARNING: module dahdi_vpmadt032_loader uses symbol param_ops_int from namespace ahdi-linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> > WARNING: module dahdi_vpmadt032_loader uses symbol __init_waitqueue_head from namespace ux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> > ...
> >
> > The fundamental issue appears to be that read_dump() is passing a
> > pointer to a statically allocated buffer for the namespace which is
> > reused as the file is parsed.
>
> Hi Shaun,
>
> Thanks for working on this. I think you are right about the root cause
> of this. I will have a closer look at your fix later today.

Thanks Matthias.

> > @@ -672,7 +696,6 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
> > unsigned int crc;
> > enum export export;
> > bool is_crc = false;
> > - const char *name, *namespace;
> >
> > if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
> > strstarts(symname, "__ksymtab"))
> > @@ -744,9 +767,13 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
> > default:
> > /* All exported symbols */
> > if (strstarts(symname, "__ksymtab_")) {
> > + const char *name, *namespace;
> > +
> > name = symname + strlen("__ksymtab_");
> > namespace = sym_extract_namespace(&name);
> > sym_add_exported(name, namespace, mod, export);
> > + if (namespace)
> > + free((char *)name);
>
> This probably should free namespace instead.

Given the implementation of sym_extract_namespace below, I believe
free((char *)name) is correct.

static const char *sym_extract_namespace(const char **symname)
{
size_t n;
char *dupsymname;

n = strcspn(*symname, ".");
if (n < strlen(*symname) - 1) {
dupsymname = NOFAIL(strdup(*symname));
dupsymname[n] = '\0';
*symname = dupsymname;
return dupsymname + n + 1;
}

return NULL;
}

I agree that freeing name instead of namespace is a little surprising
unless you know the implementation of sym_extract_namespace.

I thought about changing the the signature of sym_extract_namespace() to
make it clear when the symname is used to return a new allocation or
not, and given your comment, perhaps I should have.

\
 
 \ /
  Last update: 2019-09-30 23:22    [W:0.150 / U:9.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site