Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [RFC] module: signature infrastructure | Date | Wed, 05 Sep 2012 09:49:55 +0930 |
| |
Lucas De Marchi <lucas.de.marchi@gmail.com> writes: > Hi Rusty, > > On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell <rusty@rustcorp.com.au> wrote: >> @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct module *mod, >> } >> #endif >> >> -/* Sets info->hdr and info->len. */ >> +#ifdef CONFIG_MODULE_SIG >> +static int module_sig_check(struct load_info *info, >> + void *mod, unsigned long *len) >> +{ >> + int err; >> + unsigned long i, siglen; >> + char *sig = NULL; >> + >> + /* This is not a valid module: ELF header is larger anyway. */ >> + if (*len < sizeof(MODULE_SIG_STRING)) >> + return -ENOEXEC; >> + >> + for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) { >> + /* Our memcmp is dumb, speed it up a little. */ >> + if (((char *)mod)[i] != MODULE_SIG_STRING[0]) >> + continue; > > Since the signature is appended to the module, why don't you go > backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and > making this first comparison?
We've had this discussion multiple times. Simple wins. It's so marginal, I don't really care, but I've changed it to:
int err; unsigned long i, siglen, markerlen; char *sig = NULL;
markerlen = strlen(MODULE_SIG_STRING); /* This is not a valid module: ELF header is larger anyway. */ if (*len < markerlen) return -ENOEXEC;
for (i = *len - markerlen; i > 0; i--) { /* Our memcmp is dumb, speed it up a little. */ if (((char *)mod)[i] != MODULE_SIG_STRING[0]) continue; if (memcmp(mod+i, MODULE_SIG_STRING, markerlen)) continue;
sig = mod + i + markerlen; siglen = *len - i - markerlen; *len = i; break; }
We could also implement memrchr(), or memrmem(). Hell, if we had memmem() in the kernel I'd gladly use it.
> Or let the magic string as the last thing in the module and store the > signature length, too. In this case no scanning is needed
Yes, they did that too, but append is simpler. I don't even have to think about endianness (Dmitry chose be32) or parsing (David chose 5-digit ascii numeric encoding).
Scanning the module is the least of our issues since we've just copied it and we're about to SHA it.
Cheers, Rusty.
| |