[lkml]   [2004]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Fw: signed kernel modules?

    > I'd prefer to see:
    > err = module_verify(hdr, len, &gpgsig_ok);
    > if (err)
    > goto free_hdr;

    I've been moaned at for doing this before. Other people have told me they
    prefer to see the value returned through the return value since there's enough

    > And then have module_verify for the !CONFIG_MODULE_SIG case (in
    > module-verify.h) simply be:

    I think it should still check the ELF, even if we're not going to check a
    signature. This permits us to drop a few checks later in the module loading

    > + tmp = (size_t) hdr->e_shentsize * (size_t) hdr->e_shnum;
    > + elfcheck(tmp < size - hdr->e_shoff);
    > Multiplicative overflow.

    Not so in this ELF incarnation. The multiply parameters are both 16-bit values
    which I cast to 32-bit values before multiplying. I could, I suppose, put
    checks on this.

    I've added a check to make sure hdr->e_shnum is less than SHN_LORESERVE.

    > Also check that hdr->e_shentsize is sizeof(Elf_Shdr) since you assume that
    > below.


    > + mvdata->secsizes = kmalloc(hdr->e_shnum * sizeof(size_t), GFP_KERNEL);
    > + memset(mvdata->secsizes, 0, hdr->e_shnum * sizeof(size_t));
    > Multiplicative overflow again: we could kmalloc 0 bytes and overflow below.

    A 16-bit value multiplied by a 32/64-bit value which 4 or 8. Where's the

    Try compiling and running:

    #include <stdio.h>
    int main() { printf("%zu\n", sizeof(sizeof(char))); return 0; }

    > + secstop = mvdata->sections + mvdata->nsects;
    > Subtler multiplicative overflow.

    There's already a check in to make sure it won't overflow, given the
    additional checks to limit e_shnum (which is unsigned 16 bits) and that
    e_shentsize is correct.

    > + if (section->sh_entsize > 0)
    > + seccheck(section->sh_size % section->sh_entsize == 0);
    > Divide by zero (thanks Alan!).

    Not so. Look more closely, particularly at the if-statement.

    > I think you have to check (as above) that st_name is nul terminated
    > within size.


    > I think you can overflow here. For REL and RELA sections, you don't
    > check that sh_size is <= *secsize.

    I've added checks that the sh_entsize is what I'm expecting. There's already a
    check that the section size divides exactly by the ent-size (you claimed it
    had a div-by-0 error above).

    > That's all I found,


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 14:07    [W:0.025 / U:65.788 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site