Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes

[jasonmolenda]

Summary

I caught this while running the testsuite against lldb built with address sanitizer (ASAN) enabled - it found one problem when running the TestLinuxCore.py test. The ELFLinuxPrPsInfo structure has two fixed width strings in it, pr_fname (16 chars) and pr_psargs (80 chars). They are not required to be nul (\0) terminated, and in the case of ppc64le, pr_fname is not -

(lldb) p prpsinfo
(ELFLinuxPrPsInfo) $1 = {

pr_fname = {
  [0] = 'l'
  [1] = 'i'
  [2] = 'n'
  [3] = 'u'
  [4] = 'x'
  [5] = '-'
  [6] = 'p'
  [7] = 'p'
  [8] = 'c'
  [9] = '6'
  [10] = '4'
  [11] = 'l'
  [12] = 'e'
  [13] = '.'
  [14] = 'o'
  [15] = 'u'
}

When we copy this into a std::string,

thread_data.name = prpsinfo.pr_fname;

the read goes off the end of the array. It goes into the next element on the structure, pr_psargs, so it's unlikely to crash, but it's an easy one to fix so I think we should.

TestLinuxCore.py's do_test() could also get passed in the expected thread name and verify that it was set correctly), that would have caught this without using ASAN. But given that ASAN did catch it, I'm pretty happy with it as-is.

[labath]

Looks good, just make sure to not include extra \0 bytes.

source/Plugins/Process/elf-core/ProcessElfCore.cpp

668 ↗

(On Diff #132500)

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.

[clayborg]

source/Plugins/Process/elf-core/ProcessElfCore.cpp

668 ↗

(On Diff #132500)

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;

lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp

case LINUX::NT_PRSTATUS: {
       break;
     }
     case LINUX::NT_PRPSINFO: {
       have_prpsinfo = true;
       ELFLinuxPrPsInfo prpsinfo;
       Status status = prpsinfo.Parse(note.data, arch);
       if (status.Fail())
         return status.ToError();
-      thread_data.name = prpsinfo.pr_fname;
+      thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)));
       SetID(prpsinfo.pr_pid);
       break;
     }
     case LINUX::NT_SIGINFO: {
       ELFLinuxSigInfo siginfo;
       Status status = siginfo.Parse(note.data, arch);
       if (status.Fail())
         return status.ToError();