Re: [patch] linux 2.4.5: __dbe_table iteration #2

[Keith Owens <kaos@ocs.com.au>]

On Thu, 23 Aug 2001 18:52:45 +0200 (MET DST), 
"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> wrote:
>On Thu, 23 Aug 2001, Keith Owens wrote:
>> The definition of struct archdata in kernel and modutils can be
>> different, a new kernel layout with an old modutils is legal but fatal
>> unless you code for it.  The correct test for archdata is
>> 
>> if (!mod_member_present(mp, archdata_start) ||
>>     (mp->archdata_end - mp->archdata_start) <=
>>      offsetof(struct archdata, dbe_table_end))
>> 	continue;
> Hmm, your suggested code checks if the passed struct is long enough for
>dbe_table_start only -- what about dbe_table_end?  The following code: 

If archdata_end-archdata_start <= offsetof(dbe_table_end) then archdata
is too small to contain the first byte of dbe_table_end, skip the
archdata.  If archdata is large enough to contain the first byte of
dbe_table_end, assume that it contains all of dbe_table_end.

>While modutils as released won't ever pass a smaller
>struct, someone may modify them or use another program to invoke
>init_module(), so we need to protect the kernel against bogus data. 

You have to be root to call init_module() or run insmod, root can do a
lot more damage than passing an invalid structure size.  This type of
test is not to catch malicious code, it is to detect that the user is
running an old modutils with a smaller archdata than the kernel can
handle.

You are correct that modutils as released will never pass a smaller
archdata struct for mips so strictly speaking this test is superfluous.
However this type of code gets cut and pasted so I want size checking
on all archdata, when it is copied the developer has to think about
changing the test instead of forgetting to add a test.

Your suggested code works just as well but is less readable.  Go with
either.

>> The rest of the code looks OK, except it needs a global change of
>> arch_init_module: to module_arch_init: to match the macro name.
>
> OK, I'll do it.  It should have been done for ia64 in the first place.
>Or should it be changed into "<arch>_init_module" to match functions' real
>names?

Leave it as module_arch_init, it tells us how the code was called.

>> Do you have the corresponding modutils patch or shall I do it? 
>
> I've send it to you separately just after the kernel patch.  Should I
>resend it? 

No thanks, I found it.