[Lldb-commits] [PATCH] D42828: Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:668
         return status.ToError();
-      thread_data.name = prpsinfo.pr_fname;
+      thread_data.name.assign (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname));
       SetID(prpsinfo.pr_pid);
----------------
labath wrote:
> In case the name *is* null-terminated, this will forcibly include the \0 bytes into the string, which is not good.
> I think this should be something like `assign(pr_fname, strnlen(pr_fname, sizeof(pf_fname))` (maybe there is a more c++-y way of writing that, but I couldn't think of one).
> 
> I think adding a check for the thread name in the test still has value (asan will just check we don't do anything stupid, but it won't verify we actually produce the right name in the end), but I can do that as a follow-up.
This is how we cap segment lengths on ObjectFileMachO:
```
ConstString const_segname(load_cmd.segname, std::min<size_t>(strlen(load_cmd.segname), sizeof(load_cmd.segname)));
```

This won't keep ASAN happy though as the strlen might go off the end. You might be able to check the last byte since I am guessing they will zero fill by default:

```
if (prpsinfo.pr_fname[sizeof (prpsinfo.pr_fname)-1] != '\0')
  thread_data.name.assign (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname));
else
  thread_data.name = prpsinfo.pr_fname;
```


Repository:
  rL LLVM

https://reviews.llvm.org/D42828