Apply patch to fix RHEL-189247

Resolves: RHEL-189247

Signed-off-by: Adrian Reber <areber@redhat.com>
This commit is contained in:
Adrian Reber 2026-07-01 17:27:53 +02:00
parent f108a12fde
commit f435649fea
2 changed files with 202 additions and 1 deletions

193
3071.patch Normal file
View File

@ -0,0 +1,193 @@
From bb22b4105c7e9ee6b89b740fe716db442375c296 Mon Sep 17 00:00:00 2001
From: Adrian Reber <areber@redhat.com>
Date: Fri, 26 Jun 2026 19:30:02 +0000
Subject: [PATCH] cgroup: fix stack buffer overflow in ctrl_dir_and_opt()
ctrl_dir_and_opt() builds comma-separated controller directory and
mount option strings by repeatedly appending via snprintf() into
stack-allocated buffers. The offsets (doff, ooff) are advanced by
the snprintf() return value, but snprintf() returns the number of
bytes that *would* have been written on truncation, not the number
actually written. When a controller name is long enough to cause
truncation, the offset advances past the buffer end. On the next
iteration, the remaining-length calculation (e.g. ds - doff) goes
negative and is implicitly cast to a large size_t, disabling the
truncation guard entirely. Subsequent snprintf() calls and the
trailing-comma removal then write out of bounds on the stack
(CWE-121: Stack-based Buffer Overflow).
A crafted cgroup.img with oversized cnames entries triggers this
during restore in any code path that calls ctrl_dir_and_opt():
prepare_cgns(), move_in_cgroup(), prepare_cgroup_properties(),
prepare_cgroup_sfd(), and cgroupd().
Fix this by checking every snprintf() call for truncation before
advancing the offset. Each append now verifies that remaining
space is positive and that snprintf() did not return a value
greater than or equal to the remaining space. On any overflow
the function returns -1 and all five callers now propagate this
error. Also change the type of `off` in prepare_cgroup_properties()
from unsigned int to int so the -1 return is not silently converted
to a large positive value.
This vulnerability was identified and analyzed by an AI security
analysis tool.
Assisted-by: claude-opus-4-6
Signed-off-by: Adrian Reber <areber@redhat.com>
---
criu/cgroup.c | 75 +++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 67 insertions(+), 8 deletions(-)
diff --git a/criu/cgroup.c b/criu/cgroup.c
index d1f65df343..91547f87ed 100644
--- a/criu/cgroup.c
+++ b/criu/cgroup.c
@@ -1058,10 +1058,24 @@ int dump_cgroups(void)
return ret;
}
+/*
+ * Build the comma-separated controller directory name and mount option
+ * strings from a CgControllerEntry. Returns the length written into
+ * @dir (without the trailing comma, which is replaced by '\0'), or -1
+ * if the output would overflow the supplied buffers.
+ *
+ * Every snprintf() call is checked for truncation: snprintf() returns
+ * the number of bytes that *would* have been written, so blindly
+ * adding that to the offset can move it past the buffer end. The
+ * next append would then compute a negative remaining length, which
+ * wraps to a huge size_t, removing the truncation guard and causing
+ * a stack buffer overflow (CWE-121).
+ */
static int ctrl_dir_and_opt(CgControllerEntry *ctl, char *dir, int ds, char *opt, int os)
{
int i, doff = 0, ooff = 0;
bool none_opt = false;
+ int ret, rem;
for (i = 0; i < ctl->n_cnames; i++) {
char *n;
@@ -1070,25 +1084,58 @@ static int ctrl_dir_and_opt(CgControllerEntry *ctl, char *dir, int ds, char *opt
if (strstartswith(n, "name=")) {
n += 5;
if (opt && !none_opt) {
- ooff += snprintf(opt + ooff, os - ooff, "none,");
+ rem = os - ooff;
+ if (rem <= 0)
+ goto overflow;
+ ret = snprintf(opt + ooff, rem, "none,");
+ if (ret < 0 || ret >= rem)
+ goto overflow;
+ ooff += ret;
none_opt = true;
}
}
+ rem = ds - doff;
+ if (rem <= 0)
+ goto overflow;
+
if (n[0] == 0)
- doff += snprintf(dir + doff, ds - doff, "unified,");
+ ret = snprintf(dir + doff, rem, "unified,");
else
- doff += snprintf(dir + doff, ds - doff, "%s,", n);
- if (opt)
- ooff += snprintf(opt + ooff, os - ooff, "%s,", ctl->cnames[i]);
+ ret = snprintf(dir + doff, rem, "%s,", n);
+
+ if (ret < 0 || ret >= rem)
+ goto overflow;
+ doff += ret;
+
+ if (opt) {
+ rem = os - ooff;
+ if (rem <= 0)
+ goto overflow;
+ ret = snprintf(opt + ooff, rem, "%s,", ctl->cnames[i]);
+ if (ret < 0 || ret >= rem)
+ goto overflow;
+ ooff += ret;
+ }
}
+ /* Need at least one character written to strip the trailing ',' */
+ if (doff <= 0)
+ goto overflow;
+
/* Chop the trailing ','-s */
dir[--doff] = '\0';
- if (opt)
+ if (opt) {
+ if (ooff <= 0)
+ goto overflow;
opt[ooff - 1] = '\0';
+ }
return doff;
+
+overflow:
+ pr_err("ctrl_dir_and_opt: controller name data exceeds buffer\n");
+ return -1;
}
/* Some properties cannot be restored after the cgroup has children or tasks in
@@ -1169,6 +1216,8 @@ static int prepare_cgns(CgSetEntry *se)
}
aux_off = ctrl_dir_and_opt(ctrl, aux, sizeof(aux), NULL, 0);
+ if (aux_off < 0)
+ return -1;
/* We need to do an unshare() here as unshare() pins the root
* of the cgroup namespace to whatever the current cgroups are.
@@ -1235,6 +1284,8 @@ static int move_in_cgroup(CgSetEntry *se)
}
aux_off = ctrl_dir_and_opt(ctrl, aux, sizeof(aux), NULL, 0);
+ if (aux_off < 0)
+ return -1;
/* Note that unshare(CLONE_NEWCGROUP) doesn't change the view
* of previously mounted cgroupfses; since we're restoring via
@@ -1660,7 +1711,8 @@ static int prepare_cgroup_dir_properties(char *path, int off, CgroupDirEntry **e
int prepare_cgroup_properties(void)
{
char cname_path[PATH_MAX];
- unsigned int i, off;
+ unsigned int i;
+ int off;
for (i = 0; i < n_controllers; i++) {
CgControllerEntry *c = controllers[i];
@@ -1671,6 +1723,8 @@ int prepare_cgroup_properties(void)
}
off = ctrl_dir_and_opt(c, cname_path, sizeof(cname_path), NULL, 0);
+ if (off < 0)
+ return -1;
if (prepare_cgroup_dir_properties(cname_path, off, c->dirs, c->n_dirs) < 0)
return -1;
}
@@ -1918,7 +1972,10 @@ static int prepare_cgroup_sfd(CgroupEntry *ce)
return -1;
}
- ctl_off += ctrl_dir_and_opt(ctrl, paux + ctl_off, sizeof(paux) - ctl_off, opt, sizeof(opt));
+ ret = ctrl_dir_and_opt(ctrl, paux + ctl_off, sizeof(paux) - ctl_off, opt, sizeof(opt));
+ if (ret < 0)
+ return -1;
+ ctl_off += ret;
/* Create controller if not yet present */
if (access(paux, F_OK)) {
@@ -2037,6 +2094,8 @@ static int cgroupd(int sk)
continue;
aux_off = ctrl_dir_and_opt(ctrl, aux, sizeof(aux), NULL, 0);
+ if (aux_off < 0)
+ return -1;
format = ctrl->cnames[0][0] ? "/%s/tasks" : "/%s/cgroup.threads";
snprintf(aux + aux_off, sizeof(aux) - aux_off, format, ce->path);

View File

@ -13,7 +13,7 @@
Name: criu
Version: 4.2
Release: 4%{?dist}
Release: 5%{?dist}
Summary: Tool for Checkpoint/Restore in User-space
License: GPL-2.0-only AND LGPL-2.1-only AND MIT
URL: http://criu.org/
@ -29,6 +29,9 @@ Patch0: disable.network.locking.via.rpc.patch
# retain the full LDFLAGS set.
Patch1: 0001-build-keep-LDFLAGS-intact-filter-Wl-only-for-direct-ld.patch
# RHEL-189247
Patch2: https://patch-diff.githubusercontent.com/raw/checkpoint-restore/criu/pull/3071.patch
# Add protobuf-c as a dependency.
# We use this patch because the protobuf-c package name
# in RPM and DEB is different.
@ -111,6 +114,7 @@ This script can help to workaround the so called "PID mismatch" problem.
%setup -q
%patch -P 0 -p1
%patch -P 1 -p1
%patch -P 2 -p1
%patch -P 99 -p1
%build
@ -186,6 +190,10 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libcriu.a
%doc %{_mandir}/man1/criu-ns.1*
%changelog
* Wed Jul 01 2026 Adrian Reber <areber@redhat.com> - 4.2-5
- Apply patch to fix RHEL-189247
- Resolves: RHEL-189247
* Fri Mar 27 2026 Christopher Lusk <clusk@redhat.com> - 4.2-4
- Fix full RELRO: patch nmk build.mk to preserve -Wl flags for gcc-linked targets
- Resolves: RHEL-152222