Page MenuHomeFreeBSD

Fix a kernel panic in DTrace's rw_iswriter subroutine. On FreeBSD the sense of rw_write_held() and rw_iswriter() were reversed probably due to a cut and paste error. Using rw_iswriter() would cause the kernel to panic.
ClosedPublic

Authored by gnn on Dec 6 2016, 2:00 PM.
Tags
None
Referenced Files
F125008698: D8718.id22736.diff
Sat, Aug 2, 8:39 AM
Unknown Object (File)
Fri, Aug 1, 5:43 AM
Unknown Object (File)
Mon, Jul 7, 8:21 AM
Unknown Object (File)
Sat, Jul 5, 4:11 PM
Unknown Object (File)
Jun 25 2025, 12:46 PM
Unknown Object (File)
Jun 23 2025, 12:47 AM
Unknown Object (File)
Jun 17 2025, 4:12 PM
Unknown Object (File)
Jun 14 2025, 6:24 PM
Subscribers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 6153
Build 6409: arc lint + arc unit

Event Timeline

gnn retitled this revision from to Fix a kernel panic in DTrace's rw_iswriter subroutine. On FreeBSD the sense of rw_write_held() and rw_iswriter() were reversed probably due to a cut and paste error. Using rw_iswriter() would cause the kernel to panic..
gnn updated this object.
gnn edited the test plan for this revision. (Show Details)
markj added inline comments.
sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4394

I know this isn't a new bug, but I think this is wrong: lc_owner will set lowner to some non-NULL value even if the lock is held shared. For instance, owner_sx() sets

*owner = (struct thread *)SX_OWNER(sx->sx_lock);

and if sx has shared holders this value will be non-zero.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4394

Also this is somewhat incorrect as it calls into the kernel from DTrace probe context. That is, there's nothing preventing one from putting a probe on the lc_owner() implementation for a given lock class. Probably we could just blacklist those functions when FBT creates probes.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4394

It probably makes the most sense to split out the RW_WRITE_HELD case from the SX_EXCLUSIVE_HELD case and proceed to fix this from there.

I think the blacklisting mentioned in your second comment can be done as a separate change.

markj edited edge metadata.
markj added inline comments.
sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
4394

I'm not sure that's necessary? I think owner_sx() and owner_rw() just need to set owner to NULL (but keep the same return value as before) if the lock is held shared. That can probably be fixed separately too, so I don't see anything wrong with committing this as-is.

This revision is now accepted and ready to land.Dec 7 2016, 4:49 AM
This revision was automatically updated to reflect the committed changes.