Hi Gavin,
On Mon, 2022-11-28 at 22:26 -0800, ga...@matician.com wrote:
> Since size checking has been moved to
> dwfl_elf_phdr_memory_callback(),
> there is no longer a need for dwfl_segment_report_module() to enforce
> the same. Reading beyond the end of the dynamic section actually causes
> issues when passing the data to elfXX_xlatetom() because it is possible
> that src->d_size is not a multiple of recsize (for ELF_T_DYN, recsize is
> 16 while the minimum required alignment is 8), causing elfXX_xlatetom()
> to return ELF_E_INVALID_DATA.
I don't fully follow this logic.
The code as written doesn't seem to guarantee that
dwfl_segment_report_module will always be called with
dwfl_elf_phdr_memory_callback as memory_callback. Although it probably
will be in practice.
So you are removing this check:
> && ! read_portion (&read_state, &dyn_data, &dyn_data_size,
> start, segment, dyn_vaddr, dyn_filesz))
> {
> - /* dyn_data_size will be zero if we got everything from the initial
> - buffer, otherwise it will be the size of the new buffer that
> - could be read. */
> - if (dyn_data_size != 0)
> - dyn_filesz = dyn_data_size;
> -
Reading read_portion it shows dyn_data_size being set to zero if
read_state->buffer_available has everything (dyn_filesz) requested.
Otherwise memory_callback (we assume dwfl_elf_phdr_memory_callback) is
called with *data_size = dyn_filesz. Which will then be set to the
actual buffer size being read.
So dyn_data_size might be bigger than the dynfilesz we are requesting?
Or smaller I assume.
If you are protecting against it becoming bigger, should the check be
changed to:
if (dyn_data_size != 0 && dyn_data_size < dyn_filesz)
dyn_filesz = dyn_data_size;
?
Thanks,
Mark
