linux-kernel - Re: [PATCH 5/6] module: make modversion_info contain a pointer, not an array.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090209175050.GB9486@mediacenter>
Date:	Mon, 9 Feb 2009 11:50:50 -0600
From:	Shawn Bohrer <shawn.bohrer@...il.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	linux-kernel@...r.kernel.org, Jon Masters <jonathan@...masters.org>
Subject: Re: [PATCH 5/6] module: make modversion_info contain a pointer,
	not an array.

On Sat, Feb 07, 2009 at 12:54:28PM +1030, Rusty Russell wrote:
> On Friday 06 February 2009 02:31:28 Shawn Bohrer wrote:
> > On Thu, Jan 29, 2009 at 12:05:52AM +1030, Rusty Russell wrote:
> > > With allmodconfig (minus non-building modules) on 32-bit x86:
> > > Total size of modules before: 60009790 bytes
> > > Total size of modules after: 55927866 bytes
> > > 
> > > Saving 7% of module size for CONFIG_MODVERSIONS=y; and these sections
> > > are kept resident as well.
> > > 
> > > Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> > > ---
> > >  include/linux/module.h |    4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/module.h b/include/linux/module.h
> > > --- a/include/linux/module.h
> > > +++ b/include/linux/module.h
> > > @@ -37,10 +37,12 @@ struct kernel_symbol
> > >  	const char *name;
> > >  };
> > >  
> > > +/* This is put in the __versions section of a module to indicate the version
> > > + * it expects for unknown symbols. */
> > >  struct modversion_info
> > >  {
> > >  	unsigned long crc;
> > > -	char name[MODULE_NAME_LEN];
> > > +	char *name;
> > >  };
> > 
> > Hey Rusty thanks for the change, but I just got around to testing this
> > and I have a few observations/questions.  First this breaks:
> > 
> > modprobe --dump-modversions foo.ko
> 
> Hmm, that's an argument not to make this change.  Really, other than disk
> space benefit we get all the benefits from just dropping the 
> __versions section once loaded.
> 
> > I actually don't care, but just happened to stumble upon it when I also
> > noticed that your changes seemed to be working a little too well.  I
> > currently happen to be building my modules out of order for example
> > module B depends on symbols from module A, but I build module B first.
> > This means that the exported symbols are not in the Module.symvers file
> > when module B is compiled.  I would expect module B to fail to load yet
> > with your patches it magically works and I don't see any errors in the
> > logs.
> 
> Interesting.  What is the value of /proc/sys/kernel/tainted?

It is 3 in both cases (built in the correct and incorrect order).  If
I'm reading this correctly that means my module is proprietary and was
force loaded.  It is proprietary, but was not literally force loaded.
I'm guessing since the modversions information is wrong it sets the
force loaded bit.  I have no idea why the force loaded bit is set even
when things are built in the correct order.

Of course this made me dig a little deeper, and I think I may have been
fighting a regression all along, or possibly I'm missing a kernel config
option.  My real problem is that on openSUSE 11.1 (2.6.27) I was trying
to load my module and modprobe returned:

FATAL: Error inserting foo: Invalid module format

running dmesg I saw the kernel print:

foo: no symbol version for struct_module

After some digging I realized I wasn't building with modversion support
and that led to the long symbol name problem.  I originally believed
that openSUSE 11.1 turned on CONFIG_MODVERSIONS and previous versions
did not have it enabled.  After taking a closer look at openSUSE 10.3
(2.6.22) I found it did have CONFIG_MODVERSIONS=y and that modprobing my
module succeeds but the kernel does print:

foo: no version for "stuct_module" found: kernel tainted

This also results in a taint flag of 3.  So I'm thinking the regression
here is that somewhere between 2.6.25 (I know openSUSE 11 worked too)
and 2.6.27 the kernel stopped allowing modules to load if they didn't
build with modversion support and the kernel was built with
CONFIG_MODVERSIONS=y.

> And anyway, what was the symbol name which is over 56 characters long which
> started this?

Yeah, that is an interesting question isn't it?  Let me just say that
namespaces and name mangling make it really easy to have names longer
than 56 characters.

I actually don't care at all about modversions.  I also don't care if it
taints the kernel, I really just want the modules to load.

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