Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux: reorder /proc/<pid>/status parsing #1481

Merged
merged 1 commit into from
May 17, 2024
Merged

Conversation

cgzones
Copy link
Member

@cgzones cgzones commented May 16, 2024

In case parsing an essential pid entry file like 'status' we treat the process as a short living one and ignore it. In the relevant goto label the process structure is free'd, thus it must not have been inserted into the global process table.

Reorder parsing the status file after potentially inserting the process into the process table.

Fixes: 22d25db ("Linux: detect container process by different PID namespace")
Closes: #1455

@cgzones cgzones added bug 🐛 Something isn't working Linux 🐧 Linux related issues labels May 16, 2024
@cgzones cgzones added this to the 3.4.0 milestone May 16, 2024
Copy link
Member

@BenBE BenBE left a comment

Choose a reason for hiding this comment

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

Two minor nits …

linux/LinuxProcessTable.c Outdated Show resolved Hide resolved
linux/LinuxProcessTable.c Outdated Show resolved Hide resolved
@Explorer09
Copy link
Contributor

A minor gripe here: The commit description isn't obvious that it fixes a use-after-free. I would write the description like this:

Linux: read pid status before inserting it into process table

When parsing an essential pid entry file like 'status' fails, we treat
the process as a short living one and skip adding it into the process
table. This step should be done before the process is added, or else
the goto label can free the process structure, creating use-after-free
scenarios later on.

Fixes: 22d25db ("Linux: detect container process by different PID namespace")
Closes: htop-dev#1455

When parsing an essential pid entry file like 'status' fails, we treat
the process as a short-lived one and skip adding it into the process
table.

This should be done before the process is added, as the goto label used
for error handling can free the process structure, thus causing an
use-after-free scenario.

Fixes: 22d25db ("Linux: detect container process by different PID namespace")
Closes: htop-dev#1455
@fasterit fasterit merged commit a94e766 into htop-dev:main May 17, 2024
17 checks passed
@cgzones cgzones deleted the uaf branch May 18, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working Linux 🐧 Linux related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sudden Segementation Fault
4 participants