ctl_report_supported_opcodes: Handle invalid requested service action
ClosedPublic
Actions

Authored by jhb on Sep 9 2024, 6:14 PM.

Details

Reviewers

asomers
emaste
khorben_defora.org

Group Reviewers

cam

Commits

rG0e3a21196101: ctl_report_supported_opcodes: Handle invalid requested service action

Summary

Service actions are only valid up to 31 as they are encoded in the low
5 bits of byte 1 in CDBs. Fail requests with a requested service
action of 32 or higher with an INVALID FIELD IN COMMAND specifying
byte 4 as the illegal byte.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Sep 9 2024, 6:14 PM

Herald added a reviewer: cam. · View Herald TranscriptSep 9 2024, 6:14 PM

Herald added a subscriber: imp. · View Herald Transcript

jhb requested review of this revision.Sep 9 2024, 6:14 PM

Harbormaster completed remote builds in B59408: Diff 143151.Sep 9 2024, 6:14 PM

jhb added a reviewer: khorben_defora.org.Sep 9 2024, 6:14 PM

jhb added inline comments.

sys/cam/ctl/ctl.c
7478	Note that this is not a generic "there's an invalid field in the command", but is specifically saying that the RSO_OPTIONS field in the cdb->options byte is invalid.

asomers added inline comments.Sep 9 2024, 6:21 PM

sys/cam/ctl/ctl.c
7439	Should "field" be 4 for REQUESTED SERVICE ACTION instead of 1 for SERVICE ACTION?

khorben_defora.org added a parent revision: Restricted Differential Revision.Sep 10 2024, 10:56 PM

jhb added inline comments.Sep 14 2024, 9:38 AM

sys/cam/ctl/ctl.c
7439	Sigh, yes, and actually, it needs to be ignored for RSO_OPTIONS_ALL and RSO_OPTIONS_OC so I need to revisit this a bit.

jhb retitled this revision from ctl: Correctly reject invalid service action for REPORT SUPPORTED OPCODES to ctl_report_supported_opcodes: Handle invalid requested service action.Sep 24 2024, 4:32 PM

jhb edited the summary of this revision. (Show Details)

jhb added a parent revision: D46775: ctl_report_supported_opcodes: Rename invalid label to invalid_options.

Split up and only check RSA when relevant

Harbormaster completed remote builds in B59583: Diff 143678.Sep 24 2024, 4:33 PM

Reduce diff

Harbormaster completed remote builds in B59584: Diff 143680.Sep 24 2024, 4:34 PM

jhb marked an inline comment as done.Sep 24 2024, 4:35 PM

jhb added inline comments.

sys/cam/ctl/ctl.c
7439	This should now be fixed.
7469–7477	Removing the SA5 check here is on purpose, for the ASA case both with and without SA5 is fine and is handled below.