[PATCH V1] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[PATCH V1] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Robert Yang]

* V2
  - Also check data_list.data.d.d_buf before free scn->data_base, this can fix
    prelink error libqb_1.0.3 on mips and mips64.

* V1
  - Initial version

Robert Yang (1):
  libelf/elf_end.c: check data_list.data.d.d_buf before free it

 libelf/elf_end.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH V2] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Robert Yang]

The one which actually saves the data is data_list.data.d.d_buf, so check it
before free rawdata_base.

This can fix a segmentation fault when prelink libqb_1.0.3:
prelink: /usr/lib/libqb.so.0.18.2: Symbol section index outside of section numbers

The segmentation fault happens when prelink call elf_end().

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 libelf/elf_end.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libelf/elf_end.c b/libelf/elf_end.c
index 160f0b8..5280a70 100644
--- a/libelf/elf_end.c
+++ b/libelf/elf_end.c
@@ -160,14 +160,16 @@ elf_end (Elf *elf)
 		   architecture doesn't require overly stringent
 		   alignment the raw data buffer is the same as the
 		   one used for presenting to the caller.  */
-		if (scn->data_base != scn->rawdata_base)
+		if ((scn->data_base != scn->rawdata_base)
+		    && (scn->data_list.data.d.d_buf != NULL))
 		  free (scn->data_base);
 
 		/* The section data is allocated if we couldn't mmap
 		   the file.  Or if we had to decompress.  */
-		if (elf->map_address == NULL
+		if ((elf->map_address == NULL
 		    || scn->rawdata_base == scn->zdata_base
 		    || (scn->flags & ELF_F_MALLOCED) != 0)
+		    && (scn->data_list.data.d.d_buf != NULL))
 		  free (scn->rawdata_base);
 
 		/* Free the list of data buffers for the section.
-- 
2.7.4

Re: [PATCH V2] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Mark Wielaard]

On Wed, Aug 29, 2018 at 04:53:20PM +0800, Robert Yang wrote:
> The one which actually saves the data is data_list.data.d.d_buf, so check it
> before free rawdata_base.
> 
> This can fix a segmentation fault when prelink libqb_1.0.3:
> prelink: /usr/lib/libqb.so.0.18.2: Symbol section index outside of section numbers
> 
> The segmentation fault happens when prelink call elf_end().

Are you sure this isn't a bug in prelink like we discussed last time?
If it isn't, can you give a short example how this issue happens?

Thanks,

Mark

Re: [PATCH V2] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Robert Yang]

On 08/31/2018 03:57 AM, Mark Wielaard wrote:
> On Wed, Aug 29, 2018 at 04:53:20PM +0800, Robert Yang wrote:
>> The one which actually saves the data is data_list.data.d.d_buf, so check it
>> before free rawdata_base.
>>
>> This can fix a segmentation fault when prelink libqb_1.0.3:
>> prelink: /usr/lib/libqb.so.0.18.2: Symbol section index outside of section numbers
>>
>> The segmentation fault happens when prelink call elf_end().
> 
> Are you sure this isn't a bug in prelink like we discussed last time?
> If it isn't, can you give a short example how this issue happens?

Sorry, I can't make sure which ones is wrong, libqb, prelink or elfutils, this
happens when cross compiling, and I've built more than 4 hunderds of packages,
libqb 1.0.3 is the only package which has the problem, I've also fixed prelink,
but it is another segmentation fault error. I've reported this problem to libqb
community, then they make another branch for libqb, and it works well without
any errors, the branch is topic-no-ldsection, and the commit is:
https://github.com/ClusterLabs/libqb/commit/358e0120d8cd288095907869d3f8da92937188a0

I've used gdb/valgrind to debug this segfault, but can't find prelink's distinct
problem, the only problem I found is that elfutil's elf_end() free() a NULL
memory, so I made this patch.

I think that someone who uses libqb_1.0.3 + elfutils + prelink + crosscompile
would meet the same problem.

// Robert

> 
> Thanks,
> 
> Mark
> 

Re: [PATCH V2] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Mark Wielaard]

Hi Robert,

On Fri, 2018-08-31 at 10:17 +0800, Robert Yang wrote:
> Sorry, I can't make sure which ones is wrong, libqb, prelink or
> elfutils, this
> happens when cross compiling, and I've built more than 4 hunderds of packages,
> libqb 1.0.3 is the only package which has the problem, I've also fixed prelink,
> but it is another segmentation fault error. I've reported this problem to libqb
> community, then they make another branch for libqb, and it works well without
> any errors, the branch is topic-no-ldsection, and the commit is:
> https://github.com/ClusterLabs/libqb/commit/358e0120d8cd288095907869d3f8da92937188a0

So, this is a separate issue? Or does the prelink problem also go away
when using that commit/branch?

> I've used gdb/valgrind to debug this segfault, but can't find prelink's distinct
> problem, the only problem I found is that elfutil's elf_end() free() a NULL
> memory, so I made this patch.

OK. So I believe that is because prelink's error handling seems wrong.
It seems to assume it adding the ELF data buffer itself, so frees it,
but the data actually seemed to come from elf_getdata, so shouldn't
have been freed by prelink.

Thanks,

Mark

Re: [PATCH V2] libelf/elf_end.c: check data_list.data.d.d_buf before free it

[Robert Yang]

On 08/31/2018 05:35 PM, Mark Wielaard wrote:
> Hi Robert,
> 
> On Fri, 2018-08-31 at 10:17 +0800, Robert Yang wrote:
>> Sorry, I can't make sure which ones is wrong, libqb, prelink or
>> elfutils, this
>> happens when cross compiling, and I've built more than 4 hunderds of packages,
>> libqb 1.0.3 is the only package which has the problem, I've also fixed prelink,
>> but it is another segmentation fault error. I've reported this problem to libqb
>> community, then they make another branch for libqb, and it works well without
>> any errors, the branch is topic-no-ldsection, and the commit is:
>> https://github.com/ClusterLabs/libqb/commit/358e0120d8cd288095907869d3f8da92937188a0
> 
> So, this is a separate issue? Or does the prelink problem also go away
> when using that commit/branch?
> 
>> I've used gdb/valgrind to debug this segfault, but can't find prelink's distinct
>> problem, the only problem I found is that elfutil's elf_end() free() a NULL
>> memory, so I made this patch.
> 
> OK. So I believe that is because prelink's error handling seems wrong.
> It seems to assume it adding the ELF data buffer itself, so frees it,
> but the data actually seemed to come from elf_getdata, so shouldn't
> have been freed by prelink.

Thanks, I will investigate that.

// Robert

> 
> Thanks,
> 
> Mark
>