chroot: chroot the path that --skip-chdir validated - #13678
Conversation
--skip-chdir is only permitted when NEWROOT resolves to '/'. The guard canonicalized NEWROOT to check that, then chroot(2) was called on the original spelling. A NEWROOT symlink repointed between the two would pass the guard and still land the process elsewhere, with the working directory left outside it because --skip-chdir suppresses the chdir. Since the guard only succeeds when the resolution is '/', chrooting the resolved path is the same destination and closes the window. NEWROOT is kept for diagnostics so the error still names what the caller asked for. GNU has the same pattern (is_root() canonicalizes, chroot(newroot) does not), so this is hardening rather than a compatibility fix. All five NEWROOT forms (plain /, symlink to /, symlink to a jail, a real jail, a nonexistent path) produce byte-identical output and exit codes to GNU 9.11.130 before and after, and the error text still quotes './lnroot' rather than '/'. Reported by the Cantina audit as GHSA-h57h-p7mr-8j5f.
Merging this PR will degrade performance by 2.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | numfmt_to_si_precision[10000] |
91.9 ms | 96.2 ms | -4.5% |
| ❌ | Simulation | numfmt_from_si[10000] |
80.9 ms | 84.7 ms | -4.43% |
| ❌ | Simulation | numfmt_to_si[10000] |
89.3 ms | 92.6 ms | -3.51% |
| ❌ | Simulation | numfmt_round_modes[("up", 10000)] |
90.7 ms | 93.9 ms | -3.46% |
| ❌ | Simulation | numfmt_round_modes[("down", 10000)] |
91.1 ms | 94.4 ms | -3.44% |
| ❌ | Simulation | numfmt_round_modes[("towards-zero", 10000)] |
91.2 ms | 94.4 ms | -3.44% |
| ❌ | Simulation | numfmt_padding[(10000, 50)] |
95.1 ms | 98.4 ms | -3.3% |
| ⚡ | Simulation | cksum_crc32b |
41.4 ms | 39.1 ms | +5.79% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing sylvestre:chroot-skip-chdir-use-validated-path (16dbc6f) with main (a0ad917)2
Footnotes
-
46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(4cd8608) during the generation of this report, so a0ad917 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
--skip-chdir is only permitted when NEWROOT resolves to '/'. The guard canonicalized NEWROOT to check that, then chroot(2) was called on the original spelling. A NEWROOT symlink repointed between the two would pass the guard and still land the process elsewhere, with the working directory left outside it because --skip-chdir suppresses the chdir.