lkml.org 
[lkml]   [2004]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: Fw: signed kernel modules?
    From
    Date
    On Tue, 2004-10-12 at 18:35, David Woodhouse wrote:
    > We know _precisely_ what the kernel looks at -- we wrote its linker. It
    > really isn't that hard.

    Write the code. Then come back and tell me it "isn't that hard".

    Let me make this clear: I refuse to include any solution which doesn't
    protect against accidental, as well as deliberate, corruption. This
    means your "canonicalization" code has to be very, very paranoid about
    not trusting the data until the signature is verified. The current code
    does very simple checks then completely trusts the module contents,
    especially the section headers: to make signatures worth anything, your
    code must not do this.

    Here's the level of paranoia required for the simplest case, that of
    signing the entire module. The last Howells patches I saw didn't even
    do any of *this*, let alone checking the rest of the module:

    +static int in_range(Elf_Ehdr *hdr,
    + unsigned long len,
    + unsigned long offset,
    + unsigned long elemsize,
    + unsigned long num)
    +{
    + /* We're careful with wrap here. */
    + if (offset > len)
    + return 0;
    + if (elemsize * num / num != elemsize)
    + return 0;
    + if (elemsize * num > len - offset)
    + return 0;
    + return 1;
    +}
    +
    +/* Lots of checking: we don't trust anything until signature matched. */
    +static int check_modsig(Elf_Ehdr *hdr, unsigned long len)
    +{
    + Elf_Shdr *sechdrs;
    + unsigned long i, stroff;
    +
    + /* Section headers must all be in range. */
    + if (!in_range(hdr, len, hdr->e_shoff, sizeof(Elf_Shdr), hdr->e_shnum))
    + return -EINVAL;
    +
    + /* Index of section which contains headers must be good. */
    + if (hdr->e_shstrndx >= hdr->e_shnum)
    + return -EINVAL;
    +
    + sechdrs = (void *)hdr + hdr->e_shoff;
    + stroff = sechdrs[hdr->e_shstrndx].sh_offset;
    +
    + for (i = 1; i < hdr->e_shnum; i++) {
    + if (!in_range(hdr, len, stroff+sechdrs[i].sh_name, 1,
    + sizeof("module_sig")))
    + continue;
    + if (strcmp((char *)hdr + stroff+sechdrs[i].sh_name,
    + "module_sig") != 0)
    + continue;
    + if (!in_range(hdr, len, sechdrs[i].sh_offset,
    + sechdrs[i].sh_size, 1))
    + return -EINVAL;
    + return calc_signature(hdr, len, sechdrs[i].sh_offset,
    + sechdrs[i].sh_size);
    + }
    + tainted |= TAINT_FORCED_MODULE;
    + return 0;
    +}
    +#else
    +static int check_modsig(Elf_Ehdr *hdr, unsigned int len)
    +{
    + return 0;
    +}
    +#endif /* CONFIG_MODULE_SIG */
    +
    #ifdef CONFIG_SMP
    /* Number of blocks used and allocated. */
    static unsigned int pcpu_num_used, pcpu_num_allocated;
    @@ -1522,6 +1614,9 @@ static struct module *load_module(void _
    if (len < hdr->e_shoff + hdr->e_shnum * sizeof(Elf_Shdr))
    goto truncated;

    + if ((err = check_modsig(hdr, len)) != 0)
    + goto free_hdr;
    +
    /* Convenience variables */
    sechdrs = (void *)hdr + hdr->e_shoff;
    secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
    > > Nor do I have to re-iterate the points from the discussion for someone
    > > who hasn't bothered reading it. But I did.
    >
    > Sorry, I didn't think the discussion had been in public. While I'm sure
    > I _could_ read mail in David's inbox, I feel it would be somewhat
    > impolite. It's not that I "haven't bothered". :)

    Sorry, thought you were CC'd the whole time. My mistake.

    Rusty.


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

    \
     
     \ /
      Last update: 2005-03-22 14:06    [W:0.026 / U:0.568 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site