cam: Zero bio pointer in user-supplied SCSI CCBs
ClosedPublic
Actions

Authored by cem on Nov 8 2016, 8:15 PM.

Details

Reviewers

scottl
markj
trasz
imp

Commits

rS308451: cam: Zero bio pointer in user-supplied SCSI CCBs

Summary

The BUF_TRACKING bio pointer only makes sense for kernel consumers of CCBs.

PR: 214250
Reported by: trasz@

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

cem updated this revision to Diff 22096.Nov 8 2016, 8:15 PM

cem retitled this revision from to cam: Zero bio pointer in user-supplied SCSI CCBs The BUF_TRACKING bio pointer only makes sense for kernel consumers of CCBs..

cem updated this object.

cem edited the test plan for this revision. (Show Details)

cem added reviewers: trasz, imp, scottl, markj.

cem retitled this revision from cam: Zero bio pointer in user-supplied SCSI CCBs The BUF_TRACKING bio pointer only makes sense for kernel consumers of CCBs. to cam: Zero bio pointer in user-supplied SCSI CCBs.Nov 8 2016, 8:17 PM

cem updated this object.

Though I wonder why only the SCSI_IOs?

This revision is now accepted and ready to land.Nov 8 2016, 8:20 PM

markj added inline comments.Nov 8 2016, 8:21 PM

sys/cam/cam_xpt.c
418 ↗	(On Diff #22096)	Why not do this for all CCBs?

In D8477#176243, @imp wrote:

Though I wonder why only the SCSI_IOs?

That's just all we added the tracking to in r308155. It could be made more generic by a motivated person :-).

sys/cam/cam_xpt.c
418 ↗	(On Diff #22096)	Only XPT_SCSI_IO CCBs have valid `inccb->csio` members. For other CCBs that would scribble some random struct member.

markj accepted this revision.Nov 8 2016, 8:28 PM

markj edited edge metadata.

scottl added inline comments.Nov 8 2016, 8:29 PM

sys/cam/cam_xpt.c
418 ↗	(On Diff #22096)	What about XPT_ATA_IO? I guess this is a question more for the previous commit; it looks like XPT_ATA_IO was omitted from that too. Next question, since i wasn't on the review for the original commit, is why can't this be done with dtrace?

cem added inline comments.Nov 8 2016, 9:15 PM

sys/cam/cam_xpt.c
418 ↗	(On Diff #22096)	What about XPT_ATA_IO? I guess this is a question more for the previous commit; it looks like XPT_ATA_IO was omitted from that too. Yeah. Same answer as to Warner — we were only interested in SCSI. No reason it has to be exclusive to SCSI CCBs, though. Next question, since i wasn't on the review for the original commit, is why can't this be done with dtrace? I'm not sure it can be done in dtrace — the main utility of this tracking is for debugging panics. Can you access dtrace state from a panic or core? Even if you can, the overhead of dtrace is likely significantly higher.