Skip to content

Commit

Permalink
stalld: Fix freeing of invalid pointer
Browse files Browse the repository at this point in the history
If wrong regex is given, then while parsing
  regex we call cleanup_regex
  - parse_task_ignore_string()
   - compile_regex()
    - cleanup_regex()

compiled = *compiled_expr;
if (compiled)
        free(compiled);
- we doesn't reset the *compiled_expr variable after freeing
  and it is extern variable
- again cleanup_regex() gets called from stalld.c
  with already freed address(Non Null Address):

- when we `systemctl restart stalld`, we get coredump:
which shows
systemd[1]: Stopping Stall Monitor...
systemd-coredump[780991]: Process 780670 (stalld) of user 0 dumped core.
  Stack trace of thread 780670:
  #0  0x00007f6becf1e041 raise (libc.so.6 + 0x3d041)
  #1  0x00007f6becf07536 abort (libc.so.6 + 0x26536)
  #2  0x00007f6becf5f5a8 n/a (libc.so.6 + 0x7e5a8)
  #3  0x00007f6becf66fea n/a (libc.so.6 + 0x85fea)
  #4  0x00007f6becf673dc n/a (libc.so.6 + 0x863dc)
  #5  0x000055666563ed7e n/a (/usr/bin/stalld (deleted) + 0x6d7e)
systemd[1]: stalld.service: Main process exited, code=dumped, status=6/ABRT
systemd[1]: stalld.service: Failed with result 'core-dump'

- Resetting the extern variable to NULL after
  freeing the memory solves this issue.

Change-Id: Ia6bae7379970b5633194619d00c1d19adea120c2
Signed-off-by: Ankit Jain <ankitja@vmware.com>
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/19968
Tested-by: gerrit-photon <photon-checkins@vmware.com>
Reviewed-by: Srivatsa S. Bhat <srivatsab@vmware.com>
  • Loading branch information
jaankit authored and Srivatsa S. Bhat committed Mar 10, 2023
1 parent 15ca6df commit ae30361
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
From 6ca2e32c1b134cf16f4a705f6422b1899879c596 Mon Sep 17 00:00:00 2001
From: Ankit Jain <ankitja@vmware.com>
Date: Mon, 6 Mar 2023 08:56:05 +0000
Subject: [PATCH] stalld:utils: Fix freeing of invalid pointer

If wrong regex is given, then while parsing
regex we call cleanup_regex
- parse_task_ignore_string()
- compile_regex()
- cleanup_regex()

compiled = *compiled_expr;
if (compiled)
free(compiled);
- we doesn't reset the *compiled_expr variable after freeing
and it is extern variable
- again cleanup_regex() gets called from stalld.c
with already freed address(Non Null Address):
https://git.kernel.org/pub/scm/utils/stalld/stalld.git/tree/src/stalld.c?h=v1.18.0#n1307

- when we `systemctl restart stalld`, we get coredump:
which shows
systemd[1]: Stopping Stall Monitor...
systemd-coredump[780991]: Process 780670 (stalld) of user 0 dumped core.
Stack trace of thread 780670:
#0 0x00007f6becf1e041 raise (libc.so.6 + 0x3d041)
#1 0x00007f6becf07536 abort (libc.so.6 + 0x26536)
#2 0x00007f6becf5f5a8 n/a (libc.so.6 + 0x7e5a8)
#3 0x00007f6becf66fea n/a (libc.so.6 + 0x85fea)
#4 0x00007f6becf673dc n/a (libc.so.6 + 0x863dc)
#5 0x000055666563ed7e n/a (/usr/bin/stalld (deleted) + 0x6d7e)
systemd[1]: stalld.service: Main process exited, code=dumped, status=6/ABRT
systemd[1]: stalld.service: Failed with result 'core-dump'

- Resetting the extern variable to NULL after
freeing the memory solves this issue.

Signed-off-by: Ankit Jain <ankitja@vmware.com>
---
src/utils.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/src/utils.c b/src/utils.c
index 589756f..bfe178c 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -371,6 +371,7 @@ void cleanup_regex(unsigned int *nr_task, regex_t **compiled_expr)
regfree(&compiled[i]);
}
free(compiled);
+ *compiled_expr = NULL;
}
*nr_task = 0;
}
--
2.23.1

5 changes: 4 additions & 1 deletion SPECS/stalld/stalld.spec
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Summary: Daemon that finds starving tasks in the system and gives them a temporary boost
Name: stalld
Version: 1.14.1
Release: 5%{?dist}
Release: 6%{?dist}
License: GPLv2
Group: System/Tools
URL: https://git.kernel.org/pub/scm/utils/stalld/stalld.git
Expand Down Expand Up @@ -33,6 +33,7 @@ Patch13: 0001-throttling-Always-null-terminate-sched_rt_runtime_us.patch
Patch14: 0001-stalld-print-process-comm-and-cpu-when-boosting.patch
Patch15: 0001-stalld-Fix-memory-leak-in-print_boosted_info.patch
Patch16: 0001-stalld-Detect-runnable-dying-tasks.patch
Patch17: 0001-stalld-utils-Fix-freeing-of-invalid-pointer.patch

%description
The stalld program monitors the set of system threads, looking for
Expand Down Expand Up @@ -79,6 +80,8 @@ rm -rf %{buildroot}
%license %{_datadir}/licenses/%{name}/gpl-2.0.txt

%changelog
* Mon Mar 06 2023 Ankit Jain <ankitja@vmware.com> 1.14.1-6
- Fix freeing of invalid pointer
* Fri Feb 17 2023 Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu> 1.14.1-5
- Detect and boost runnable dying tasks
* Wed Nov 16 2022 Keerthana K <keerthanak@vmware.com> 1.14.1-4
Expand Down

0 comments on commit ae30361

Please sign in to comment.