Hi,

On Tue, May 24, 2022 at 11:53:11PM +0800, Di Chen via Elfutils-devel wrote:
> All the request changes are fixed, ready for review again.
> 
> 1. help message updated: "Use the dynamic segment when possible for
> displaying info"
> 2. move enum dyn_idx to a proper place
> 3. add strtab_data's NULL check in function: handle_dynamic()
> 4. add phdr's NULL check in function: print_dynamic()
> 5. add comments for function: find_offsets()
> 6. remove redundant return-statement in function: get_dynscn_addrs()
> 7. add run-readelf-Dd.sh to EXTRA_DISTS
> 8. check strsz in (dyn->d_un.d_ptr < strtab_data->d_size) in function:
> handle_dynamic()

Sorry the re-review took so long. This looks great. I did add a NEWS
entry and wrote a Changelog entry while re-reviewing. And a few small
whitespace fixups.

The only code change I made was:

-      char *lib_name = NULL;
-
-      if (!use_dynamic_segment)
-    lib_name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val);
-      else if (use_dynamic_segment)
-    lib_name = ((char *)strtab_data->d_buf) + dyn->d_un.d_ptr;
-      else
-    break;
+      char *name = NULL;
+      if (dyn->d_tag == DT_NEEDED
+         || dyn->d_tag == DT_SONAME
+         || dyn->d_tag == DT_RPATH
+         || dyn->d_tag == DT_RUNPATH)
+       {
+         if (! use_dynamic_segment)
+           name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val);
+         else if (dyn->d_un.d_ptr < strtab_data->d_size
+                  && memrchr (strtab_data->d_buf + strtab_data->d_size - 1, '\0',
+                              strtab_data->d_size - 1 - dyn->d_un.d_ptr) != NULL)
+           name = ((char *) strtab_data->d_buf) + dyn->d_un.d_ptr;
+       }

That does the check whether dyn->d_un.d_ptr is valid early, so it
doesn't need to be checked in each case statement. Also it adds an
extra memrchr check to make sure the string is zero terminated.

Pushed with those changes.

Thanks,

Mark