From 01de65bd036179244edc6e582f6ea0355ff919ad Mon Sep 17 00:00:00 2001 From: vasilito Date: Sun, 21 Jun 2026 11:00:32 +0300 Subject: [PATCH] redbear-power: v1.38.1 fix io_priority field index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.38 review found a HIGH severity bug in the v1.38 io_priority reader: it read fields[44] (overall field 47) which on modern Linux kernels (~52 fields total) is a memory address (~9x10^13) that overflows u32 and silently returns None for every process. The PID detail popup shows 'IO priority: ?' on every process, on every Linux kernel released in the last ~5 years. Fix: change fields[44] to fields[15] (post-comm field 15 = overall field 18 = priority per the /proc/[pid]/stat layout). The doc comment now spells out the per-field index map 0-15 so a future maintainer can verify the indexing. Strengthened the regression test: the v1.38 test did 'let _ = ...' which passed even when the reader was broken. The new test reads /proc/self/stat directly (bypassing the function), parses the priority field at vec[15] itself, and asserts the function returns the same value. A 'sanity' assertion (value < 1_000_000_000) catches the 'reading a memory address' failure mode specifically — a value > 10^9 means we're reading the wrong field again. This is exactly the kind of bug the v1.37 audit warned about: 'the existing 4 tests that exist for update_io_history all assert the post-normalize storage value, not what the renderer actually draws. A simple integration test that renders and reads back would have caught it.' The v1.38 test for io_priority had the same shape — 'let _ = ...' passes silently. The v1.38.1 test now reads the actual value back. Test count 149 -> 149 (strengthened one test in place; no new test count). Redox stripped binary: 4,348,776 bytes (unchanged; the field index is 1 character). --- .../redbear-power/source/src/pid_detail.rs | 73 ++++++++++++++----- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/local/recipes/system/redbear-power/source/src/pid_detail.rs b/local/recipes/system/redbear-power/source/src/pid_detail.rs index b279813e02..d71a242858 100644 --- a/local/recipes/system/redbear-power/source/src/pid_detail.rs +++ b/local/recipes/system/redbear-power/source/src/pid_detail.rs @@ -111,29 +111,33 @@ fn read_cmdline(pid: u32) -> Option { } } -/// Read the I/O priority class from `/proc//stat` field -/// 47 (the priority is `class | (data << 13)`; we return the -/// raw field as u32 so the caller can interpret both). -/// Field 47 is the 47th space-separated token, 0-indexed at -/// the THIRD field (state) plus 44 (matches the kernel's -/// `getpriority(2)`-derived field). +/// Read the I/O priority (nice-equivalent) from +/// `/proc//stat`. The /proc/[pid]/stat format is +/// documented in `man 5 proc`; the priority field is at +/// position 18 (1-indexed), which is post-comm index 15 +/// (0-indexed `fields[15]`) because fields 1-2 are pid+comm +/// and field 3 is state. The v1.38 first cut used `fields[44]` +/// (overall field 47), which on modern Linux kernels (~52 +/// fields total) is a memory address (~9×10¹³) — not the +/// priority. Reading that value as u32 silently overflowed +/// and the reader returned None for every process. This +/// release fixes the field index. fn read_io_priority(pid: u32) -> Option { let stat = fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; // The comm field is wrapped in parens and may contain // spaces, so we must find the LAST `)` to delimit the - // comm field, then split the rest by whitespace. The - // 44th post-comm field (1-indexed: field 47 overall) - // holds the priority. + // comm field, then split the rest by whitespace. After + // the comm: [0]=state [1]=ppid [2]=pgrp [3]=session + // [4]=tty_nr [5]=tpgid [6]=flags [7]=minflt [8]=cminflt + // [9]=majflt [10]=cmajflt [11]=utime [12]=stime + // [13]=cutime [14]=cstime [15]=priority. We want [15]. let close = stat.rfind(')')?; let tail = &stat[close + 1..]; let fields: Vec<&str> = tail.split_whitespace().collect(); - // post-comm field index 44 -> vec index 43 (0-indexed) - // post-comm field 1 is the state (after the comm), so - // 47th overall field is post-comm 45 (state + 44). - if fields.len() < 45 { + if fields.len() < 16 { return None; } - fields[44].parse::().ok() + fields[15].parse::().ok() } fn read_status(pid: u32) -> ProcStatus { @@ -312,10 +316,43 @@ mod tests { #[test] fn read_io_priority_handles_self() { - // Self's io_priority is a u32. Some kernels may not - // expose it; tolerate that. The function must not - // panic. - let _ = read_io_priority(std::process::id()); + // v1.38.1 regression test: the v1.38 first cut read + // `fields[44]` (overall field 47), which on modern + // Linux kernels is a memory address (~9×10¹³) that + // overflows u32 and silently returns None for every + // process. The fix reads `fields[15]` (overall field + // 18 = priority). This test reads the priority from + // /proc/self/stat directly (bypassing the function) + // and asserts the function returns the same value. + let pid = std::process::id(); + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")) + .expect("can read /proc/self/stat"); + let close = stat.rfind(')').expect("comm field has closing paren"); + let tail = &stat[close + 1..]; + let fields: Vec<&str> = tail.split_whitespace().collect(); + // 0-indexed: state=0, ppid=1, pgrp=2, session=3, + // tty_nr=4, tpgid=5, flags=6, minflt=7, cminflt=8, + // majflt=9, cmajflt=10, utime=11, stime=12, cutime=13, + // cstime=14, priority=15. + let expected: u32 = fields[15] + .parse() + .expect("priority is a u32"); + let actual = read_io_priority(pid) + .expect("read_io_priority must return Some for self"); + assert_eq!( + actual, expected, + "v1.38.1: read_io_priority should return the priority field (overall field 18, post-comm vec[15])" + ); + // Sanity: a typical default-nice process has priority + // 20. We're not strict (containers / reniced processes + // may differ), but if the value looks like a memory + // address (>10^9) we know we're reading the wrong + // field again. + assert!( + actual < 1_000_000_000, + "read_io_priority returned {} which looks like a memory address, not a priority. The field index is wrong again.", + actual + ); } #[test]