Skip to content

Conversation

@oontvoo
Copy link
Member

@oontvoo oontvoo commented Sep 18, 2025

If pr_name is longer than 16, it would be a non-null terminated string. Assigning it to std::string m_executable_name would cause an overflow read. Instead, just copy the name from thread_data.name.

To repro, run the elf-core/TestLinuxCore.py with asan
(Question: why is the new variable needed in the first place? can't the thread_data.name be used?)

If `pr_name` is longer than 16, it would be a non-null terminated string.
Assigning it to `std::string m_executable_name` would cause an overflow read.
Instead, just copy the name from thread_data.name.

(Question: why is the new variable needed in the first place? can't the thread_data.name be used?)
@oontvoo oontvoo requested review from GeorgeHuyubo and dmpots and removed request for GeorgeHuyubo and JDevlieghere September 18, 2025 14:41
@llvmbot llvmbot added the lldb label Sep 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 18, 2025

@llvm/pr-subscribers-lldb

Author: Vy Nguyen (oontvoo)

Changes

If pr_name is longer than 16, it would be a non-null terminated string. Assigning it to std::string m_executable_name would cause an overflow read. Instead, just copy the name from thread_data.name.

(Question: why is the new variable needed in the first place? can't the thread_data.name be used?)


Full diff: https://github.com/llvm/llvm-project/pull/159588.diff

1 Files Affected:

  • (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+1-1)
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 8f5f1242116f5..38bf13543c617 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -952,7 +952,7 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) {
         return status.ToError();
       thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)));
       SetID(prpsinfo.pr_pid);
-      m_executable_name = prpsinfo.pr_fname;
+      m_executable_name = thread_data.name;
       break;
     }
     case ELF::NT_SIGINFO: {

@oontvoo oontvoo merged commit 09e0f1e into llvm:main Sep 18, 2025
9 of 11 checks passed
Copy link
Contributor

@dmpots dmpots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. The new variable m_executable_name is used to save the name so that we can find the NT_FILE entries for the main executable, which is handled in a different part of the code.

There is the same fix in #159375.

@oontvoo
Copy link
Member Author

oontvoo commented Sep 18, 2025

Thanks for the fix. The new variable m_executable_name is used to save the name so that we can find the NT_FILE entries for the main executable, which is handled in a different part of the code.

There is the same fix in #159375.

Oops, my bad - hadn't noticed the existing fix. This was breaking our internal build so I thought I'd just merge the fix quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants