Page MenuHomeFreeBSD

Return EINVAL for userland CCBs with CAM_UNLOCKED set
ClosedPublic

Authored by markj on Mar 2 2017, 7:19 PM.

Details

Summary

Userland cannot correctly set CAM_UNLOCKED; it is internal to the
implementation and not defined in the CAM spec. Thus, return EINVAL if
it's set in the CAMIOCOMMAND or CAMIOQUEUE argument.

I also noticed that some error paths in the CAMIOQUEUE handler don't
free the request or relock the periph, so fix that too.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj retitled this revision from to Return EINVAL for userland CCBs with CAM_UNLOCKED set.
markj edited the test plan for this revision. (Show Details)
markj updated this object.
mav edited edge metadata.

It looks fine, except this flag is probably not the only one that should be banned.

This revision is now accepted and ready to land.Mar 2 2017, 7:27 PM
In D9869#203728, @mav wrote:

It looks fine, except this flag is probably not the only one that should be banned.

I looked for others, but didn't see any obvious candidates. Many CCB flags appear to have no handling at all in the kernel, and most of the rest appear to be used in some way by camcontrol.

ken edited edge metadata.

I agree with mav. This looks good, there might be other flags that we should ban as well if they come in from user land. But this is a good start.

This revision was automatically updated to reflect the committed changes.

-r314624 prevented a PowerMac G5 so-called "Quad Core" from competing
its boot (of -r314687): CAM status: Command timeout's. Reverting the two
files fixed it. See, e.g.,

https://lists.freebsd.org/pipermail/svn-src-head/2017-March/097981.html

-r314624 prevented a PowerMac G5 so-called "Quad Core" from competing
its boot (of -r314687): CAM status: Command timeout's. Reverting the two
files fixed it. See, e.g.,

https://lists.freebsd.org/pipermail/svn-src-head/2017-March/097981.html

This turned out to be apparently unrelated to the commit.