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.