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
Unknown Object (File)
Sun, Dec 8, 6:04 PM
Unknown Object (File)
Wed, Dec 4, 9:22 AM
Unknown Object (File)
Nov 21 2024, 9:33 AM
Unknown Object (File)
Nov 15 2024, 8:55 AM
Unknown Object (File)
Nov 2 2024, 8:29 AM
Unknown Object (File)
Nov 2 2024, 8:29 AM
Unknown Object (File)
Nov 2 2024, 8:29 AM
Unknown Object (File)
Nov 2 2024, 8:21 AM
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.