Hi,
When adding data to existing sections in ELF files, libelf may corrupt those sections, i.e. overwrite the existing data if certain conditions are met.
If an Elf_Scn structure has seen a call to elf_rawdata(scn) before but no call to elf_getdata(scn), scn->read_data flag is set, but not scn->data_list_rear. Thus, elf_newdata(scn) incorrectly detects a "new user added section" when really it is a section with live, valid data that will be overwritten by elf_update(), corrupting the section.
This patch fixes this incorrect behaviour.
Signed-off-by: Thilo Schulz thilo@tjps.eu --- libelf/elf_newdata.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libelf/elf_newdata.c b/libelf/elf_newdata.c index 90d1813..f90eb0a 100644 --- a/libelf/elf_newdata.c +++ b/libelf/elf_newdata.c @@ -64,7 +64,7 @@ elf_newdata (Elf_Scn *scn)
rwlock_wrlock (scn->elf->lock);
- if (scn->data_read && scn->data_list_rear == NULL) + if (scn->data_read && scn->data_list_rear == NULL && !scn->rawdata.s) { /* This means the section was created by the user and this is the first data. */
Hi,
On Mon, 2014-06-09 at 21:05 +0200, Thilo Schulz wrote:
When adding data to existing sections in ELF files, libelf may corrupt those sections, i.e. overwrite the existing data if certain conditions are met.
If an Elf_Scn structure has seen a call to elf_rawdata(scn) before but no call to elf_getdata(scn), scn->read_data flag is set, but not scn->data_list_rear.
Do you happen to have a small testcase that shows the buggy behavior?
Thus, elf_newdata(scn) incorrectly detects a "new user added section" when really it is a section with live, valid data that will be overwritten by elf_update(), corrupting the section.
This patch fixes this incorrect behaviour.
I was wondering whether we want to check scn->rawdata.s directly, or if we could rely on ELF_F_FILEDATA being set for scn->flags?
Thanks,
Mark
On Tuesday 10 June 2014 11:48:15 Mark Wielaard wrote:
On Mon, 2014-06-09 at 21:05 +0200, Thilo Schulz wrote:
When adding data to existing sections in ELF files, libelf may corrupt those sections, i.e. overwrite the existing data if certain conditions are met.
If an Elf_Scn structure has seen a call to elf_rawdata(scn) before but no call to elf_getdata(scn), scn->read_data flag is set, but not scn->data_list_rear.
Do you happen to have a small testcase that shows the buggy behavior?
Sure. This is an excerpt from the final program. A short word on what it is supposed to do in my practical application: I am doing a project for the AVR platform, which is mixed C/assembly. For command parser functions that are called out of assembly into C code, I need to replace the final return statements with rjmps back to program code in an object file generated from my assembly. So I want to add new symbols and relocations to a cmd.o file.
Find as attachment a simple program, which checks for the presence of the two symbols cmd_response and cmd_noresponse and adds them if they don't exist. Since the AVR architecture is 8 bit, I am only using Elf32_ structures throughout, so you may need to compile test .o objects with -m32 for the test program to work on them.
I was wondering whether we want to check scn->rawdata.s directly, or if we could rely on ELF_F_FILEDATA being set for scn->flags?
Seems reasonable though I don't know the code as well as you do I guess.
As a further note: A similar bug, albeit for slightly different reasons, occurs when adding relocations. Adding a relocation with elf_newdata() then elf_update() results in the old data being "forgotten" if there was no elf_getdata() call before to load that data into memory. The cause is a bit different because in this case, there was not a call to elf_rawdata() before and this still happened. I imagine, this might also be a problem for string tables.
On Tue, 2014-06-10 at 15:31 +0200, Thilo Schulz wrote:
On Tuesday 10 June 2014 11:48:15 Mark Wielaard wrote:
On Mon, 2014-06-09 at 21:05 +0200, Thilo Schulz wrote:
When adding data to existing sections in ELF files, libelf may corrupt those sections, i.e. overwrite the existing data if certain conditions are met.
If an Elf_Scn structure has seen a call to elf_rawdata(scn) before but no call to elf_getdata(scn), scn->read_data flag is set, but not scn->data_list_rear.
Do you happen to have a small testcase that shows the buggy behavior?
Sure. This is an excerpt from the final program.
Thanks. It think that shows the second bug you describe. It helped me write a specific test case for both issues (attached).
I was wondering whether we want to check scn->rawdata.s directly, or if we could rely on ELF_F_FILEDATA being set for scn->flags?
Seems reasonable though I don't know the code as well as you do I guess.
I wish I understood the code very well :) But now that I wrote the testcase and you pointed out the second bug, I am not sure of the fix anymore. It does seem to fix the first issue, but then you immediately hit the second.
As a further note: A similar bug, albeit for slightly different reasons, occurs when adding relocations. Adding a relocation with elf_newdata() then elf_update() results in the old data being "forgotten" if there was no elf_getdata() call before to load that data into memory. The cause is a bit different because in this case, there was not a call to elf_rawdata() before and this still happened. I imagine, this might also be a problem for string tables.
Indeed. The attached testcase shows both issues. Calling elf_getdata() and then elf_newdata() works as expected. But elf_newdata drops all existing data when elf_rawdata is called before and elf_newdata keeps the size, but not the actual content bytes of existing data of a section if elf_getdata isn't called before.
Still scratching my head a little how to resolve both issues properly.
Thanks,
Mark
On Thursday 12 June 2014 14:30:52 Mark Wielaard wrote:
Thanks. It think that shows the second bug you describe.
First bug. My testcase first uses elf_strptr() to get the strings for the symbols, and that func internally calls elf_rawdata() thus triggering the first bug described.
Still scratching my head a little how to resolve both issues properly.
Hmm.. I didn't do any debugging on the second bug reported, but it seems to me that once a section was marked dirty, outputting to a new elf causes it to only consider data explicitly added to the lib internal memory representation of the elf. It _did_ write out the data I added via elf_newdata(), and entered the appropriate offsets into the header file, but completely omitted copying of the original data.
These issues can all be worked around with by explicitly calling elf_getdata().
Still, I'm a bit baffled that a bug like this could exist for so long in .. well I guess these are kind of core developer utils, right? Or maybe I'm doing some really sick stuff noone else dreamt up yet ;)
On Thu, 2014-06-12 at 14:30 +0200, Mark Wielaard wrote:
On Tue, 2014-06-10 at 15:31 +0200, Thilo Schulz wrote:
I was wondering whether we want to check scn->rawdata.s directly, or if we could rely on ELF_F_FILEDATA being set for scn->flags?
Seems reasonable though I don't know the code as well as you do I guess.
I wish I understood the code very well :) But now that I wrote the testcase and you pointed out the second bug, I am not sure of the fix anymore. It does seem to fix the first issue, but then you immediately hit the second.
As a further note: A similar bug, albeit for slightly different reasons, occurs when adding relocations. Adding a relocation with elf_newdata() then elf_update() results in the old data being "forgotten" if there was no elf_getdata() call before to load that data into memory. The cause is a bit different because in this case, there was not a call to elf_rawdata() before and this still happened. I imagine, this might also be a problem for string tables.
Indeed. The attached testcase shows both issues. Calling elf_getdata() and then elf_newdata() works as expected. But elf_newdata drops all existing data when elf_rawdata is called before and elf_newdata keeps the size, but not the actual content bytes of existing data of a section if elf_getdata isn't called before.
Still scratching my head a little how to resolve both issues properly.
Sorry this took 9 months... But I believe these issues have finally been resolved in current git elfutils. At least my testcase now works as expected. Hope it also now works for your code.
Cheers,
Mark
elfutils-devel@lists.fedorahosted.org
-
Mark Wielaard -
Thilo Schulz