RFR: 8298267: Too many conversion specifiers in CgroupV1Subsystem::pids_max_val
Thomas Stuefe
stuefe at openjdk.org
Wed Dec 7 13:44:44 UTC 2022
On Wed, 7 Dec 2022 12:40:13 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Trivial fix.
>
> This code:
>
> ```c++
> GET_CONTAINER_INFO_CPTR(cptr, _pids, "/pids.max",
> "Maximum number of tasks is: %s", "%s %*d", pidsmax, 1024);
>
>
> Expands to this call:
>
> ```c++
> // matchline = NULL
> err = subsystem_file_line_contents(_pids, "/pids.max", NULL, "%s %*d", pidsmax);
>
>
> Which in turn hits this branch:
>
> ```c++
> if (matchline == NULL) {
> // single-line file case
> int matched = sscanf(p, scan_fmt, returnval);
> found_match = (matched == 1);
> }
>
>
> Now we're calling `sscanf()` with `scan_fmt = "%s %*d"`, this is undefined behavior as the number of conversion specifiers are larger than the number of pointers provided.
>
> This is the correct fix, because the file `pids.max` only contains a number. This is supported by:
>
> 1. Checking my own `pids.max` and
> 2. This [documentation](https://www.kernel.org/doc/Documentation/cgroup-v1/pids.txt) suggests that it only consists of a number by recommending altering the file through `echo 2 > /sys/fs/cgroup/pids/parent/pids.max`
Even if pid_max only matches the first %s, the error is benign since sscanf stops at the first non-matching element, avoiding filling in the non-existent destination for %d.
The question is whether the V1 pid controller provides pids_max to fit "%s %d". I'm not sure, I'm not the original author.
According to https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v1/pids.html, pids_max can contain a number or the string "max". So I think the "%*d" could probably be removed, if only to avoid confusion. But I leave that up to @jerboaa, he knows this code better than me.
-------------
PR: https://git.openjdk.org/jdk/pull/11557
More information about the hotspot-runtime-dev
mailing list