lkml.org 
[lkml]   [2012]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH 1/2] binfmt_elf: Fix corner case kfree of uninitialized data
    On Wed, 19 Sep 2012 15:44:35 +0100
    Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

    > Could do with double checking...
    >
    > From: Alan Cox <alan@linux.intel.com>
    >
    > If elf_core_dump is called and fill_note_info fails in the kmalloc then
    > it returns 0 but has not yet initialised all the needed fields. As a result
    > we do a kfree(randomness) after correctly skipping the thread data.
    >
    > Signed-off-by: Alan Cox <alan@linux.intel.com>
    > ---
    >
    > fs/binfmt_elf.c | 4 +++-
    > 1 file changed, 3 insertions(+), 1 deletion(-)
    >
    > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
    > index 1b4efbc..bf6d82b 100644
    > --- a/fs/binfmt_elf.c
    > +++ b/fs/binfmt_elf.c
    > @@ -1492,8 +1492,10 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
    > info->thread = NULL;
    >
    > psinfo = kmalloc(sizeof(*psinfo), GFP_KERNEL);
    > - if (psinfo == NULL)
    > + if (psinfo == NULL) {
    > + info->psinfo.data = NULL; /* So we don't free this wrongly */
    > return 0;
    > + }
    >
    > fill_note(&info->psinfo, "CORE", NT_PRPSINFO, sizeof(*psinfo), psinfo);

    afaict it's NotABug, because fill_note_info() does

    info->thread = NULL;

    psinfo = kmalloc(sizeof(*psinfo), GFP_KERNEL);
    if (psinfo == NULL) {

    and free_note_info() does

    struct elf_thread_core_info *threads = info->thread;
    while (threads) {

    so free_note_info() won't enter the freeing loop at all.

    Which is just as well, because info->thread_notes is uninitialised at
    this time.


    It all looks rather fragile - I'm wondering if it would be sanest to
    memset `info' right at the outset in elf_core_dump(), then weed out all
    the now-unneeded zeroizings in fill_note_info().


    Also, how irritating is it that fill_note_info() has a local var
    `psinfo' which has a different type from info->psinfo. That had me
    running around for a while...


    \
     
     \ /
      Last update: 2012-09-27 02:01    [W:4.378 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site