kernel: holdingsleep asks who holds the lock, not whether anyone does
int r = lk->locked; // before
int r = lk->locked && lk->pid == myproc()->pid; // after
Seven callers, and every one of them is an assertion of the form
if (!holdingsleep(&b->lock)) panic("brelse");
whose entire purpose is to catch a process releasing a lock it never took.
Answering the weaker question disarms all of them: iunlock() on an inode locked
by another process, and brelse() on somebody else's buffer, both sailed through
and unlocked it underneath the real holder. Two processes then believe they hold
the same inode, and the corruption surfaces somewhere else entirely -- which is
exactly the shape of bug these panics exist to prevent.
Found while hunting the ls -lR hang. It is not proven to be that hang's cause,
and the instrumentation in the next commits says it is probably not. It is a
real defect regardless, and it re-arms the detector that would catch the rest.
myproc() is null on the boot path before the scheduler runs and acquiresleep()
stores 0 for that case, so this compares the same way rather than dereferencing
null. Checked that nothing touches a buffer that early, so the stricter
assertion cannot fire spuriously on boot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQ7Qx8E1q1M1bbCq8QLHS2
This commit is contained in:
parent
57aff40228
commit
3eed58fe9b
1 changed files with 20 additions and 1 deletions
|
|
@ -45,11 +45,30 @@ releasesleep(struct sleeplock *lk)
|
||||||
release(&lk->lk);
|
release(&lk->lk);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Does *this process* hold lk?
|
||||||
|
//
|
||||||
|
// The pid comparison is not decoration. Without it this answers "is anyone
|
||||||
|
// holding this", and every caller is an assertion of the form
|
||||||
|
//
|
||||||
|
// if (!holdingsleep(&b->lock)) panic("brelse");
|
||||||
|
//
|
||||||
|
// whose entire purpose is to catch a process releasing a lock it never took.
|
||||||
|
// Answering the weaker question disarms all seven of them: iunlock() on an
|
||||||
|
// inode locked by another process, and brelse() on somebody else's buffer,
|
||||||
|
// both sailed through and unlocked it underneath the real holder. Two
|
||||||
|
// processes then believe they hold the same inode, and the corruption surfaces
|
||||||
|
// somewhere else entirely -- which is exactly the shape of bug these panics
|
||||||
|
// exist to prevent.
|
||||||
|
//
|
||||||
|
// myproc() is null on the boot path before the scheduler runs, and
|
||||||
|
// acquiresleep() stores 0 for that case, so compare the same way rather than
|
||||||
|
// dereferencing null here.
|
||||||
int
|
int
|
||||||
holdingsleep(struct sleeplock *lk)
|
holdingsleep(struct sleeplock *lk)
|
||||||
{
|
{
|
||||||
|
struct proc *p = myproc();
|
||||||
acquire(&lk->lk);
|
acquire(&lk->lk);
|
||||||
int r = lk->locked;
|
int r = lk->locked && lk->pid == (p ? p->pid : 0);
|
||||||
release(&lk->lk);
|
release(&lk->lk);
|
||||||
return r;
|
return r;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue