linux-kernel - RE: [PATCH] coredump: prevent double-free on an error path in core dumper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 12 Sep 2012 17:15:27 +0530
From:	Venu Byravarasu <vbyravarasu@...dia.com>
To:	Denys Vlasenko <vda.linux@...glemail.com>,
	Oleg Nesterov <oleg@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Roland McGrath <roland@...k.frob.com>
Subject: RE: [PATCH] coredump: prevent double-free on an error path in core
 dumper


> -----Original Message-----
> From: linux-kernel-owner@...r.kernel.org [mailto:linux-kernel-
> owner@...r.kernel.org] On Behalf Of Denys Vlasenko
> Sent: Wednesday, September 12, 2012 5:07 PM
> To: Oleg Nesterov; linux-kernel@...r.kernel.org; Andrew Morton; Roland
> McGrath
> Cc: Denys Vlasenko
> Subject: [PATCH] coredump: prevent double-free on an error path in core
> dumper
> 
> In !CORE_DUMP_USE_REGSET case, if elf_note_info_init fails to allocate
> memory
> for info->fields, it frees already allocated stuff and returns
> error to its caller, fill_note_info. Which in turn returns
> error to its caller, elf_core_dump. Which jumps to cleanup
> label and calls free_note_info, which will happily try to
> free all info->fields again. BOOM.
> 
> This is the fix.
> 
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>
> Signed-off-by: Denys Vlasenko <vda.linux@...glemail.com>
> ---
>  fs/binfmt_elf.c |   19 ++++---------------
>  1 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 760d7f5..1b4efbc 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1695,30 +1695,19 @@ static int elf_note_info_init(struct elf_note_info
> *info)
>  		return 0;
>  	info->psinfo = kmalloc(sizeof(*info->psinfo), GFP_KERNEL);

Why don't you change kmalloc to devm_kzalloc, so that free can be ignored altogether. 

>  	if (!info->psinfo)
> -		goto notes_free;
> +		return 0;
>  	info->prstatus = kmalloc(sizeof(*info->prstatus), GFP_KERNEL);
>  	if (!info->prstatus)
> -		goto psinfo_free;
> +		return 0;
>  	info->fpu = kmalloc(sizeof(*info->fpu), GFP_KERNEL);
>  	if (!info->fpu)
> -		goto prstatus_free;
> +		return 0;
>  #ifdef ELF_CORE_COPY_XFPREGS
>  	info->xfpu = kmalloc(sizeof(*info->xfpu), GFP_KERNEL);
>  	if (!info->xfpu)
> -		goto fpu_free;
> +		return 0;
>  #endif
>  	return 1;
> -#ifdef ELF_CORE_COPY_XFPREGS
> - fpu_free:
> -	kfree(info->fpu);
> -#endif
> - prstatus_free:
> -	kfree(info->prstatus);
> - psinfo_free:
> -	kfree(info->psinfo);
> - notes_free:
> -	kfree(info->notes);
> -	return 0;
>  }
> 
>  static int fill_note_info(struct elfhdr *elf, int phdrs,
> --
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/