libelf: Free and clear rawdata_base and zdata_base consistently
Commit Message
There could be a leak if a program called elf_strptr on a compressed
section, but the program never requests the (uncompressed) section data,
but does explicitly (re)compress that same section data.
Fix this by explicitly always freeing and clearing the zdata_base
and rawdata_base in __libelf_reset_rawdata and elf_compress. Also
clear zdata_base in elf_end so the pointer isn't indeterminate when
it is being used in a later comparison against rawdata_base.
* libelf/elf_compress.c (elf_compress): Explicitly free
zdata_base before clearing.
(__libelf_reset_rawdata): Free zdata_base if it isn't
(going to be) used for rawdata_base. Explicitly clear
rawdata_base and zdata_base after free.
* libelf/elf_end.c (elf_end): Clear zdata_base after free.
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
libelf/elf_compress.c | 14 +++++++++++++-
libelf/elf_end.c | 5 ++++-
2 files changed, 17 insertions(+), 2 deletions(-)
Comments
Hi,
On Sun, Jul 30, 2023 at 05:44:20PM +0200, Mark Wielaard wrote:
> There could be a leak if a program called elf_strptr on a compressed
> section, but the program never requests the (uncompressed) section data,
> but does explicitly (re)compress that same section data.
>
> Fix this by explicitly always freeing and clearing the zdata_base
> and rawdata_base in __libelf_reset_rawdata and elf_compress. Also
> clear zdata_base in elf_end so the pointer isn't indeterminate when
> it is being used in a later comparison against rawdata_base.
>
> * libelf/elf_compress.c (elf_compress): Explicitly free
> zdata_base before clearing.
> (__libelf_reset_rawdata): Free zdata_base if it isn't
> (going to be) used for rawdata_base. Explicitly clear
> rawdata_base and zdata_base after free.
> * libelf/elf_end.c (elf_end): Clear zdata_base after free.
Pushed.
Mark
@@ -1,5 +1,6 @@
/* Compress or decompress a section.
Copyright (C) 2015, 2016 Red Hat, Inc.
+ Copyright (C) 2023, Mark J. Wielaard <mark@klomp.org>
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -523,10 +524,20 @@ __libelf_reset_rawdata (Elf_Scn *scn, void *buf, size_t size, size_t align,
if (scn->data_base != scn->rawdata_base)
free (scn->data_base);
scn->data_base = NULL;
+ if (scn->zdata_base != buf
+ && scn->zdata_base != scn->rawdata_base)
+ {
+ free (scn->zdata_base);
+ scn->zdata_base = NULL;
+ }
if (scn->elf->map_address == NULL
|| scn->rawdata_base == scn->zdata_base
|| (scn->flags & ELF_F_MALLOCED) != 0)
- free (scn->rawdata_base);
+ {
+ free (scn->rawdata_base);
+ scn->rawdata_base = NULL;
+ scn->zdata_base = NULL;
+ }
scn->rawdata_base = buf;
scn->flags |= ELF_F_MALLOCED;
@@ -677,6 +688,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
data around, but since that might have been multiple Elf_Data
buffers let the user uncompress it explicitly again if they
want it to simplify bookkeeping. */
+ free (scn->zdata_base);
scn->zdata_base = NULL;
return 1;
@@ -157,7 +157,10 @@ elf_end (Elf *elf)
rawdata_base. If it is already used it will be
freed below. */
if (scn->zdata_base != scn->rawdata_base)
- free (scn->zdata_base);
+ {
+ free (scn->zdata_base);
+ scn->zdata_base = NULL;
+ }
/* If the file has the same byte order and the
architecture doesn't require overly stringent