Download raw body.
patch: relax ni_pledge panic
Hi,
The following patch relax the panic("ni_pledge") check in namei().
It was initially done to ensure coverage of all NDINIT() calls reachable
while pledged (it is a programming error to call namei() without
ni_pledge if pledged).
But with a race, it is possible to a unreachable syscall while pledged
(like acct(2) or chroot(2)) to trigger the panic.
syzbot reported the following panics:
- acct(2) : https://syzkaller.appspot.com/bug?extid=a8229b7ddd62e3a50eea
- chroot(2) : https://syzkaller.appspot.com/bug?id=f0ed88b77fd8900402613b1ebf01166049465abe
From my reading of the backtrace, it could occurs in the following
scenario:
[1] acct("/tmp/file")
- we are not pledged, so the call reach sys_acct().
NDINIT() doesn't set ni_pledge (we are not pledged),
and call namei().
- while in namei(), reach a sleeping point (for example,
pool_get(PR_WAITOK) at line 145):
kern/vfs_lookup.c
117 │ int
118 │ namei(struct nameidata *ndp)
119 │ {
...
140 │ /*
141 │ * Get a buffer for the name to be translated, and copy the
142 │ * name into the buffer.
143 │ */
144 │ if ((cnp->cn_flags & HASBUF) == 0)
145 │ cnp->cn_pnbuf = pool_get(&namei_pool, PR_WAITOK);
...
[2] in another thread, pledge("stdio rpath wpath"), and returns.
the process is now pledged.
[1] resume from the sleeping point and continue
kern/vfs_lookup.c
...
200 │ if (ndp->ni_cnd.cn_flags & KERNELPATH) {
201 │ ndp->ni_cnd.cn_flags |= BYPASSUNVEIL;
202 │ } else {
203 │ error = pledge_namei(p, ndp, cnp->cn_pnbuf);
204 │ if (error)
205 │ goto fail;
206 │ }
the pledge_namei() will trigger panic("ni_pledge"). it is unexpected
that a syscall disallowed-while-pledged reach this codepath when
pledged.
I propose to relax the panic() to a simple pledge_fail(). It will make
the system still running (not panic anymore), and kill the
process. After all, at pledge_namei() time we are pledged, so it should
not continue.
The following diff does it.
Commit ID: 5296b38dc15fe733903493e84b5c88cbd65e2efb
Change ID: rrmmmquowkmqwkyllnqswrlvzyznuvuk
Author : Sebastien Marie <semarie@kapouay.eu.org> (2025-02-06 08:55:58)
Committer: Sebastien Marie <semarie@kapouay.eu.org> (2025-02-06 09:23:21)
pledge: relax panic("ni_pledge") and just error out instead of panic()
diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c
index 89281cf0a2..7463a70d0f 100644
--- a/sys/kern/kern_pledge.c
+++ b/sys/kern/kern_pledge.c
@@ -595,8 +595,16 @@
return (0);
pledge = READ_ONCE(p->p_p->ps_pledge);
+ /*
+ * ni_pledge is unset. it could be a programming bug (missing entry)
+ * or some race between pledge(2) and another syscall.
+ *
+ * in both case, don't continue: we are pledged and this namei() call
+ * is unwanted while pledged. pledge_fail() will report the error using
+ * uprintf(9).
+ */
if (ni->ni_pledge == 0)
- panic("pledge_namei: ni_pledge");
+ return (pledge_fail(p, EPERM, 0));
/*
* We set the BYPASSUNVEIL flag to skip unveil checks
Comments or OK ?
--
Sebastien Marie
patch: relax ni_pledge panic