Messages in this thread | | | Date | Wed, 26 Sep 2012 16:21:51 -0700 | From | Andrew Morton <> | Subject | Re: [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...
| |