* [PATCH v2 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions Douglas Gilbert
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
A patch titled "scsi: core: Remove the cmd field from struct
scsi_request" [ce70fd9a551af] limited the size of a SCSI
CDB (command descriptor block) to 32 bytes. While this covers
the current requirements of the sd driver, OSD users and
those sending long vendor specific cdb_s via the sg or bsg
drivers will be disappointed by this regression.
This patch adds back that support without increasing the size
of the newly renovated struct scsi_cmnd. Three accessor functions
are added to encourage kernel coders not to access
scsi_cmnd::cmnd[] directly. The fix is performed by a union with
some defensive code (i.e. a relatively harmless Test Unit Ready
SCSI command) if that union is incorrectly accessed.
If the SCSI_MAX_COMPILE_TIME_CDB_LEN define is set to 16 as
proposed by one reviewer (rather than 32 in the first revision),
then struct scsi_cmnd will 16 bytes shorter than before.
The end of include/scsi/scsi_cmnd.h contains the declaration for
the construction of a scsi_cmnd sub-object. It is a "sub-object"
because what is actually constructed is a struct request object
with the scsi_cmnd sub-object tacked onto the end of it. In a
similar fashion add an inline function for the destruction of a
scsi_cmnd sub-object (and the struct request object that
precedes it).
Various files in the scsi mid-level that used scsi_cmnd::cmnd
directly are modified to use the new accessor functions. Also
use scsi_free_cmnd() where appropriate.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debugfs.c | 3 +-
drivers/scsi/scsi_error.c | 76 ++++++++++++++---------
drivers/scsi/scsi_ioctl.c | 21 ++++---
drivers/scsi/scsi_lib.c | 75 +++++++++++++++++++----
drivers/scsi/scsi_logging.c | 11 ++--
include/scsi/scsi_cmnd.h | 116 +++++++++++++++++++++++++++++++-----
include/scsi/scsi_eh.h | 6 +-
7 files changed, 240 insertions(+), 68 deletions(-)
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index 217b70c678c3..6fe9507c0ecb 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -10,6 +10,7 @@ static const char *const scsi_cmd_flags[] = {
SCSI_CMD_FLAG_NAME(TAGGED),
SCSI_CMD_FLAG_NAME(INITIALIZED),
SCSI_CMD_FLAG_NAME(LAST),
+ SCSI_CMD_FLAG_NAME(LONG_CDB),
};
#undef SCSI_CMD_FLAG_NAME
@@ -38,7 +39,7 @@ void scsi_show_rq(struct seq_file *m, struct request *rq)
int timeout_ms = jiffies_to_msecs(rq->timeout);
char buf[80] = "(?)";
- __scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
+ __scsi_format_command(buf, sizeof(buf), scsi_cmnd_get_cdb(cmd), cmd->cmd_len);
seq_printf(m, ", .cmd=%s, .retries=%d, .result = %#x, .flags=", buf,
cmd->retries, cmd->result);
scsi_flags_show(m, cmd->flags, scsi_cmd_flags,
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cdaca13ac1f1..6fe0aeaf8837 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -530,6 +530,7 @@ static void scsi_report_sense(struct scsi_device *sdev,
enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd)
{
struct scsi_device *sdev = scmd->device;
+ const u8 *cdb = scsi_cmnd_get_cdb(scmd);
struct scsi_sense_hdr sshdr;
if (! scsi_command_normalize_sense(scmd, &sshdr))
@@ -549,7 +550,7 @@ enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd)
/* handler does not care. Drop down to default handling */
}
- if (scmd->cmnd[0] == TEST_UNIT_READY &&
+ if (cdb[0] == TEST_UNIT_READY &&
scmd->submitter != SUBMITTED_BY_SCSI_ERROR_HANDLER)
/*
* nasty: for mid-layer issued TURs, we need to return the
@@ -755,6 +756,8 @@ static void scsi_handle_queue_full(struct scsi_device *sdev)
*/
static enum scsi_disposition scsi_eh_completed_normally(struct scsi_cmnd *scmd)
{
+ const u8 *cdb = scsi_cmnd_get_cdb(scmd);
+
/*
* first check the host byte, to see if there is anything in there
* that would indicate what we need to do.
@@ -791,7 +794,7 @@ static enum scsi_disposition scsi_eh_completed_normally(struct scsi_cmnd *scmd)
*/
return SUCCESS;
case SAM_STAT_RESERVATION_CONFLICT:
- if (scmd->cmnd[0] == TEST_UNIT_READY)
+ if (cdb[0] == TEST_UNIT_READY)
/* it is a success, we probed the device and
* found it */
return SUCCESS;
@@ -985,7 +988,7 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
* @scmd: SCSI command structure to hijack
* @ses: structure to save restore information
* @cmnd: CDB to send. Can be NULL if no new cmnd is needed
- * @cmnd_size: size in bytes of @cmnd (must be <= MAX_COMMAND_SIZE)
+ * @cmnd_size: size in bytes of @cmnd (must be <= SCSI_MAX_COMPILE_TIME_CDB_LEN)
* @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
*
* This function is used to save a scsi command information before re-execution
@@ -998,6 +1001,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
{
struct scsi_device *sdev = scmd->device;
+ u8 *cdb;
/*
* We need saved copies of a number of fields - this is because
@@ -1013,12 +1017,21 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
ses->resid_len = scmd->resid_len;
ses->underflow = scmd->underflow;
ses->prot_op = scmd->prot_op;
+ ses->flags = scmd->flags;
ses->eh_eflags = scmd->eh_eflags;
+ if (unlikely(scmd->flags & SCMD_LONG_CDB)) {
+ ses->l_cdb.cdbp = scmd->l_cdb.cdbp;
+ scmd->l_cdb.cdbp = NULL;
+ ses->l_cdb.dummy_tur = 0;
+ scmd->flags &= ~SCMD_LONG_CDB;
+ } else {
+ memcpy(ses->cmnd, scmd->cmnd, scmd->cmd_len);
+ memset(scmd->cmnd, 0, scmd->cmd_len);
+ }
+ cdb = scmd->cmnd;
scmd->prot_op = SCSI_PROT_NORMAL;
scmd->eh_eflags = 0;
- memcpy(ses->cmnd, scmd->cmnd, sizeof(ses->cmnd));
- memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
memset(&scmd->sdb, 0, sizeof(scmd->sdb));
scmd->result = 0;
scmd->resid_len = 0;
@@ -1031,23 +1044,22 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
scmd->sdb.table.sgl = &ses->sense_sgl;
scmd->sc_data_direction = DMA_FROM_DEVICE;
scmd->sdb.table.nents = scmd->sdb.table.orig_nents = 1;
- scmd->cmnd[0] = REQUEST_SENSE;
- scmd->cmnd[4] = scmd->sdb.length;
- scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+ scmd->cmd_len = 6; /* bytes in REQUEST SENSE command */
+ cdb[0] = REQUEST_SENSE;
+ cdb[4] = scmd->sdb.length;
} else {
scmd->sc_data_direction = DMA_NONE;
if (cmnd) {
- BUG_ON(cmnd_size > sizeof(scmd->cmnd));
- memcpy(scmd->cmnd, cmnd, cmnd_size);
- scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+ BUG_ON(cmnd_size > SCSI_MAX_RUN_TIME_CDB_LEN);
+ scmd->cmd_len = (cmnd_size > 0) ? cmnd_size : COMMAND_SIZE(cmnd[0]);
+ cdb = scsi_cmnd_set_cdb(scmd, cmnd, scmd->cmd_len);
}
}
scmd->underflow = 0;
if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
- scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
- (sdev->lun << 5 & 0xe0);
+ cdb[1] = (cdb[1] & 0x1f) | (sdev->lun << 5 & 0xe0);
/*
* Zero the sense buffer. The scsi spec mandates that any
@@ -1069,8 +1081,19 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
/*
* Restore original data
*/
+ if (unlikely(scmd->flags & SCMD_LONG_CDB))
+ kfree(scmd->l_cdb.cdbp);
scmd->cmd_len = ses->cmd_len;
- memcpy(scmd->cmnd, ses->cmnd, sizeof(ses->cmnd));
+ scmd->flags = ses->flags;
+ if (unlikely(ses->flags & SCMD_LONG_CDB)) {
+ scmd->l_cdb.cdbp = ses->l_cdb.cdbp;
+ ses->l_cdb.cdbp = NULL;
+ scmd->l_cdb.dummy_tur = 0;
+ ses->flags &= ~SCMD_LONG_CDB;
+ } else {
+ memcpy(scmd->cmnd, ses->cmnd, ses->cmd_len);
+ memset(ses->cmnd, 0, ses->cmd_len);
+ }
scmd->sc_data_direction = ses->data_direction;
scmd->sdb = ses->sdb;
scmd->result = ses->result;
@@ -1340,13 +1363,12 @@ EXPORT_SYMBOL_GPL(scsi_eh_get_sense);
*/
static int scsi_eh_tur(struct scsi_cmnd *scmd)
{
- static unsigned char tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0};
+ static const u8 tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0};
int retry_cnt = 1;
enum scsi_disposition rtn;
retry_tur:
- rtn = scsi_send_eh_cmnd(scmd, tur_command, 6,
- scmd->device->eh_timeout, 0);
+ rtn = scsi_send_eh_cmnd(scmd, (u8 *)tur_command, 6, scmd->device->eh_timeout, 0);
SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd,
"%s return: %x\n", __func__, rtn));
@@ -1831,6 +1853,7 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
{
enum scsi_disposition rtn;
+ const u8 *cdb = scsi_cmnd_get_cdb(scmd);
/*
* if the device is offline, then we clearly just pass the result back
@@ -1924,8 +1947,7 @@ enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
* these commands if there is no device available.
* other hosts report did_no_connect for the same thing.
*/
- if ((scmd->cmnd[0] == TEST_UNIT_READY ||
- scmd->cmnd[0] == INQUIRY)) {
+ if (cdb[0] == TEST_UNIT_READY || cdb[0] == INQUIRY) {
return SUCCESS;
} else {
return FAILED;
@@ -1956,7 +1978,7 @@ enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
*/
return ADD_TO_MLQUEUE;
case SAM_STAT_GOOD:
- if (scmd->cmnd[0] == REPORT_LUNS)
+ if (cdb[0] == REPORT_LUNS)
scmd->device->sdev_target->expecting_lun_change = 0;
scsi_handle_queue_ramp_up(scmd->device);
fallthrough;
@@ -2008,7 +2030,7 @@ enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
static void eh_lock_door_done(struct request *req, blk_status_t status)
{
- blk_mq_free_request(req);
+ scsi_free_cmnd(blk_mq_rq_to_pdu(req));
}
/**
@@ -2026,19 +2048,17 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
{
struct scsi_cmnd *scmd;
struct request *req;
+ u8 *cdb;
+ static const int amr_cdb_len = 6;
req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, 0);
if (IS_ERR(req))
return;
scmd = blk_mq_rq_to_pdu(req);
- scmd->cmnd[0] = ALLOW_MEDIUM_REMOVAL;
- scmd->cmnd[1] = 0;
- scmd->cmnd[2] = 0;
- scmd->cmnd[3] = 0;
- scmd->cmnd[4] = SCSI_REMOVAL_PREVENT;
- scmd->cmnd[5] = 0;
- scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
+ cdb = scsi_cmnd_set_cdb(scmd, NULL, amr_cdb_len);
+ cdb[0] = ALLOW_MEDIUM_REMOVAL;
+ cdb[4] = SCSI_REMOVAL_PREVENT;
req->rq_flags |= RQF_QUIET;
req->timeout = 10 * HZ;
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index a480c4d589f5..10b6b8a89c15 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -245,7 +245,7 @@ static int scsi_send_start_stop(struct scsi_device *sdev, int data)
* Only a subset of commands are allowed for unprivileged users. Commands used
* to format the media, update the firmware, etc. are not permitted.
*/
-bool scsi_cmd_allowed(unsigned char *cmd, fmode_t mode)
+bool scsi_cmd_allowed(/* const */ unsigned char *cmd, fmode_t mode)
{
/* root can do any command. */
if (capable(CAP_SYS_RAWIO))
@@ -346,14 +346,15 @@ static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
struct sg_io_hdr *hdr, fmode_t mode)
{
struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
+ u8 *cdb;
if (hdr->cmd_len < 6)
return -EMSGSIZE;
- if (copy_from_user(scmd->cmnd, hdr->cmdp, hdr->cmd_len))
+ cdb = scsi_cmnd_set_cdb(scmd, NULL, hdr->cmd_len);
+ if (copy_from_user(cdb, hdr->cmdp, hdr->cmd_len))
return -EFAULT;
- if (!scsi_cmd_allowed(scmd->cmnd, mode))
+ if (unlikely(!scsi_cmd_allowed(cdb, mode)))
return -EPERM;
- scmd->cmd_len = hdr->cmd_len;
rq->timeout = msecs_to_jiffies(hdr->timeout);
if (!rq->timeout)
@@ -440,7 +441,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
return PTR_ERR(rq);
scmd = blk_mq_rq_to_pdu(rq);
- if (hdr->cmd_len > sizeof(scmd->cmnd)) {
+ if (unlikely(hdr->cmd_len > SCSI_MAX_RUN_TIME_8BIT_CDB_LEN)) {
ret = -EINVAL;
goto out_put_request;
}
@@ -483,7 +484,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
ret = scsi_complete_sghdr_rq(rq, hdr, bio);
out_put_request:
- blk_mq_free_request(rq);
+ scsi_free_cmnd(scmd);
return ret;
}
@@ -521,6 +522,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
int err;
unsigned int in_len, out_len, bytes, opcode, cmdlen;
struct scsi_cmnd *scmd;
+ u8 *cdb;
char *buffer = NULL;
if (!sic)
@@ -560,14 +562,15 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
*/
err = -EFAULT;
scmd->cmd_len = cmdlen;
- if (copy_from_user(scmd->cmnd, sic->data, cmdlen))
+ cdb = scsi_cmnd_set_cdb(scmd, NULL, cmdlen);
+ if (copy_from_user(cdb, sic->data, cmdlen))
goto error;
if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
goto error;
err = -EPERM;
- if (!scsi_cmd_allowed(scmd->cmnd, mode))
+ if (unlikely(!scsi_cmd_allowed(cdb, mode)))
goto error;
/* default. possible overridden later */
@@ -619,7 +622,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
}
error:
- blk_mq_free_request(rq);
+ scsi_free_cmnd(scmd);
error_free_buffer:
kfree(buffer);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8d18cc7e510e..e06c5556fa9e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -223,15 +223,18 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
if (IS_ERR(req))
return PTR_ERR(req);
+ scmd = blk_mq_rq_to_pdu(req);
if (bufflen) {
ret = blk_rq_map_kern(sdev->request_queue, req,
buffer, bufflen, GFP_NOIO);
if (ret)
goto out;
}
- scmd = blk_mq_rq_to_pdu(req);
scmd->cmd_len = COMMAND_SIZE(cmd[0]);
- memcpy(scmd->cmnd, cmd, scmd->cmd_len);
+ if (unlikely(!scsi_cmnd_set_cdb(scmd, cmd, scmd->cmd_len))) {
+ ret = -ENOMEM;
+ goto out;
+ }
scmd->allowed = retries;
req->timeout = timeout;
req->cmd_flags |= flags;
@@ -260,7 +263,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
sshdr);
ret = scmd->result;
out:
- blk_mq_free_request(req);
+ scsi_free_cmnd(scmd);
return ret;
}
@@ -688,6 +691,7 @@ static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result)
{
struct request_queue *q = cmd->device->request_queue;
struct request *req = scsi_cmd_to_rq(cmd);
+ const u8 *cdb = scsi_cmnd_get_cdb(cmd);
int level = 0;
enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY,
ACTION_DELAYED_RETRY} action;
@@ -737,8 +741,7 @@ static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result)
*/
if ((cmd->device->use_10_for_rw &&
sshdr.asc == 0x20 && sshdr.ascq == 0x00) &&
- (cmd->cmnd[0] == READ_10 ||
- cmd->cmnd[0] == WRITE_10)) {
+ (cdb[0] == READ_10 || cdb[0] == WRITE_10)) {
/* This will issue a new 6-byte command. */
cmd->device->use_10_for_rw = 0;
action = ACTION_REPREP;
@@ -1117,8 +1120,7 @@ static void scsi_initialize_rq(struct request *rq)
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
- memset(cmd->cmnd, 0, sizeof(cmd->cmnd));
- cmd->cmd_len = MAX_COMMAND_SIZE;
+ scsi_cmnd_set_cdb(cmd, NULL, SCSI_TEST_UNIT_READY_CDB_LEN);
cmd->sense_len = 0;
init_rcu_head(&cmd->rcu);
cmd->jiffies_at_alloc = jiffies;
@@ -1460,6 +1462,7 @@ static void scsi_complete(struct request *rq)
static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
{
struct Scsi_Host *host = cmd->device->host;
+ u8 *cdb = scsi_cmnd_get_changeable_cdb(cmd);
int rtn = 0;
atomic_inc(&cmd->device->iorequest_cnt);
@@ -1489,8 +1492,7 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
/* Store the LUN value in cmnd, if needed. */
if (cmd->device->lun_in_cdb)
- cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
- (cmd->device->lun << 5 & 0xe0);
+ cdb[1] = (cdb[1] & 0x1f) | (cmd->device->lun << 5 & 0xe0);
scsi_log_send(cmd);
@@ -1600,7 +1602,6 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
return ret;
}
- memset(cmd->cmnd, 0, sizeof(cmd->cmnd));
return scsi_cmd_to_driver(cmd)->init_command(cmd);
}
@@ -3314,3 +3315,57 @@ void scsi_build_sense(struct scsi_cmnd *scmd, int desc, u8 key, u8 asc, u8 ascq)
scmd->result = SAM_STAT_CHECK_CONDITION;
}
EXPORT_SYMBOL_GPL(scsi_build_sense);
+
+/**
+ * __scsi_cmnd_set_cdb - build buffer for SCSI command (cdb) in *scmd
+ * @scmd: pointer to scsi command for which a CDB is, or will be, set
+ * @cdb: if non-NULL it is a pointer to start of SCSI CDB that is
+ * copied into *scmd . If NULL then no copy occurs.
+ * @cdb_len: number of bytes in SCSI CDB to be held in *scmd
+ * N.B. If cdb_len==0, then if *scmd holds some heap
+ * due to a long CDB, then that heap is freed.
+ *
+ * Returns a (writable) pointer to the start of a buffer, owned by *scmd,
+ * where 'cdb' has been written to, or if 'cdb' is NULL, where cdb_len
+ * bytes of a SCSI CDB may be written. If 'cdb' is NULL then the returned
+ * buffer will be zero-ed. In the rare case of a long cdb where the
+ * kzalloc() allocating the additional buffer fails, NULL is returned.
+ **/
+u8 *__scsi_cmnd_set_cdb(struct scsi_cmnd *scmd, const u8 *cdb, u16 cdb_len)
+{
+ if (unlikely(scmd->flags & SCMD_LONG_CDB)) {
+ if (unlikely(cdb_len > SCSI_MAX_COMPILE_TIME_CDB_LEN &&
+ cdb_len <= scmd->cmd_len)) {
+ if (cdb)
+ memcpy(scmd->l_cdb.cdbp, cdb, cdb_len);
+ else
+ memset(scmd->l_cdb.cdbp, 0, cdb_len);
+ scmd->cmd_len = cdb_len;
+ return scmd->l_cdb.cdbp;
+ }
+ kfree(scmd->l_cdb.cdbp);
+ scmd->l_cdb.cdbp = NULL;
+ scmd->flags &= ~SCMD_LONG_CDB;
+ }
+ scmd->cmd_len = cdb_len;
+ if (likely(cdb_len <= SCSI_MAX_COMPILE_TIME_CDB_LEN)) {
+ if (cdb_len > 0) {
+ if (cdb)
+ memcpy(scmd->cmnd, cdb, cdb_len);
+ else
+ memset(scmd->cmnd, 0, cdb_len);
+ }
+ return scmd->cmnd;
+ }
+ scmd->l_cdb.cdbp = kzalloc(cdb_len, GFP_KERNEL);
+ if (unlikely(!scmd->l_cdb.cdbp)) {
+ scmd->cmd_len = 0;
+ return NULL;
+ }
+ scmd->flags |= SCMD_LONG_CDB;
+ scmd->l_cdb.dummy_tur = 0; /* defensive: cheap aid when testing */
+ if (cdb)
+ memcpy(scmd->l_cdb.cdbp, cdb, cdb_len);
+ return scmd->l_cdb.cdbp;
+}
+EXPORT_SYMBOL_GPL(__scsi_cmnd_set_cdb);
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index ff89de86545d..0c388e47406e 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -181,6 +181,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
{
int k;
char *logbuf;
+ const u8 *cdb = scsi_cmnd_get_cdb(cmd);
size_t off, logbuf_len;
logbuf = scsi_log_reserve_buffer(&logbuf_len);
@@ -195,8 +196,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
if (WARN_ON(off >= logbuf_len))
goto out_printk;
- off += scsi_format_opcode_name(logbuf + off, logbuf_len - off,
- cmd->cmnd);
+ off += scsi_format_opcode_name(logbuf + off, logbuf_len - off, cdb);
if (off >= logbuf_len)
goto out_printk;
@@ -213,8 +213,9 @@ void scsi_print_command(struct scsi_cmnd *cmd)
scsi_cmd_to_rq(cmd)->tag);
if (!WARN_ON(off > logbuf_len - 58)) {
off += scnprintf(logbuf + off, logbuf_len - off,
- "CDB[%02x]: ", k);
- hex_dump_to_buffer(&cmd->cmnd[k], linelen,
+ "CDB[%s%02x]: ",
+ (k > 15 ? "0x" : ""), k);
+ hex_dump_to_buffer(&cdb[k], linelen,
16, 1, logbuf + off,
logbuf_len - off, false);
}
@@ -225,7 +226,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
}
if (!WARN_ON(off > logbuf_len - 49)) {
off += scnprintf(logbuf + off, logbuf_len - off, " ");
- hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1,
+ hex_dump_to_buffer(cdb, cmd->cmd_len, 16, 1,
logbuf + off, logbuf_len - off,
false);
}
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 1e80e70dfa92..ac04245ab8e6 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -15,23 +15,33 @@ struct Scsi_Host;
/*
* MAX_COMMAND_SIZE is:
- * The longest fixed-length SCSI CDB as per the SCSI standard.
- * fixed-length means: commands that their size can be determined
- * by their opcode and the CDB does not carry a length specifier, (unlike
- * the VARIABLE_LENGTH_CMD(0x7f) command). This is actually not exactly
- * true and the SCSI standard also defines extended commands and
- * vendor specific commands that can be bigger than 16 bytes. The kernel
- * will support these using the same infrastructure used for VARLEN CDB's.
- * So in effect MAX_COMMAND_SIZE means the maximum size command scsi-ml
- * supports without specifying a cmd_len by ULD's
+ * an old define that is deprecated. Firstly it refers to a SCSI CDB
+ * (command descriptor block) that is no longer the maximum value. Plus
+ * using "command" is too general when there is a precise acronym: CDB .
+ * Please use one of the more precise defines that follow,
*/
-#define MAX_COMMAND_SIZE 16
+#define MAX_COMMAND_SIZE 16 /* old constant, should be removed */
+
+/* This value is used to size a C array sufficient to hold most cdb_s */
+#define SCSI_MAX_COMPILE_TIME_CDB_LEN 16
+
+/* Relatively safe SCSI command whose CDB is 6 zero bytes */
+#define SCSI_TEST_UNIT_READY_CDB_LEN 6
+
+/* Following used to guard against wild cmd_len values */
+#define SCSI_MAX_RUN_TIME_CDB_LEN (8 + 252)
+#define SCSI_MAX_RUN_TIME_8BIT_CDB_LEN 252
struct scsi_data_buffer {
struct sg_table table;
unsigned length;
};
+struct scsi_long_cdb {
+ u64 dummy_tur; /* when zero first 6 bytes are Test Unit Ready cdb */
+ u8 *cdbp; /* byte length in scsi_cmnd::cmd_len */
+};
+
/* embedded in scsi_cmnd */
struct scsi_pointer {
char *ptr; /* data pointer */
@@ -48,12 +58,17 @@ struct scsi_pointer {
volatile int phase;
};
-/* for scmd->flags */
+/*
+ * For scmd->flags . To maintain flag value for lifetime of scsi_cmnd
+ * sub-object, make sure flag is OR-ed into SCMD_PRESERVED_FLAGS .
+ */
#define SCMD_TAGGED (1 << 0)
#define SCMD_INITIALIZED (1 << 1)
#define SCMD_LAST (1 << 2)
+#define SCMD_LONG_CDB (1 << 3) /* indicates CDB separately on heap */
+
/* flags preserved across unprep / reprep */
-#define SCMD_PRESERVED_FLAGS (SCMD_INITIALIZED)
+#define SCMD_PRESERVED_FLAGS (SCMD_INITIALIZED | SCMD_LONG_CDB)
/* for scmd->state */
#define SCMD_STATE_COMPLETE 0
@@ -65,6 +80,18 @@ enum scsi_cmnd_submitter {
SUBMITTED_BY_SCSI_RESET_IOCTL = 2,
} __packed;
+/*
+ * The scsi_cmnd structure is fundamental to the operation of the SCSI
+ * subsystem. An object (or sub-object) of this type is created paired with
+ * a struct request (sub-)object (see include/linux/blk-mq.h). The struct
+ * request sub-object always has the lower address in memory. This is why the
+ * constructor for a scsi_cmnd object [see scsi_alloc_request() below] returns
+ * a pointer to a struct request object. The blk_mq_rq_from_pdu() and
+ * blk_mq_rq_to_pdu() inline functions are supplied by the block layer to move
+ * from a pointer to one of these sub-objects to the other.
+ * Users are encouraged to destroy this paired object with scsi_free_cmnd()
+ * function (destructor) defined below so that other clean-up can be hooked in.
+ */
struct scsi_cmnd {
struct scsi_device *device;
struct list_head eh_entry; /* entry for the host eh_abort_list/eh_cmd_q */
@@ -72,7 +99,7 @@ struct scsi_cmnd {
struct rcu_head rcu;
- int eh_eflags; /* Used by error handlr */
+ int eh_eflags; /* Used by error handler */
int budget_token;
@@ -94,7 +121,18 @@ struct scsi_cmnd {
unsigned short cmd_len;
enum dma_data_direction sc_data_direction;
- unsigned char cmnd[32]; /* SCSI CDB */
+ /*
+ * Access to a SCSI command (cdb) should be via the provided functions:
+ * scsi_cmnd_set_cdb(struct scsi_cmnd *scmd, const u8 * cdb, u16 cdb_len), or
+ * scsi_cmnd_get_cdb(const struct scsi_cmnd *scmd), or
+ * scsi_cmnd_get_changeable_cdb(struct scsi_cmnd *scmd)
+ * If a LLD doesn't support cdb_len > SCSI_MAX_COMPILE_TIME_CDB_LEN then
+ * it is safe to access cmnd[] directly.
+ */
+ union { /* selected via (scsi_cmnd::flags & SCMD_LONG_CDB) */
+ u8 cmnd[SCSI_MAX_COMPILE_TIME_CDB_LEN]; /* CDB when selector 0 */
+ struct scsi_long_cdb l_cdb; /* when selector != 0 */
+ };
/* These elements define the operation we ultimately want to perform */
struct scsi_data_buffer sdb;
@@ -152,6 +190,47 @@ static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
return cmd + 1;
}
+/* Read only access function for SCSI CDB held in a scsi_cmnd object */
+static inline const u8 *scsi_cmnd_get_cdb(const struct scsi_cmnd *scmd)
+{
+ return (scmd->flags & SCMD_LONG_CDB) ? scmd->l_cdb.cdbp : scmd->cmnd;
+}
+
+/*
+ * Writable access function to an existing SCSI CDB held in a scsi_cmnd
+ * object. Must not increase the size if the pre-existing CDB .
+ */
+static inline u8 *scsi_cmnd_get_changeable_cdb(struct scsi_cmnd *scmd)
+{
+ return (scmd->flags & SCMD_LONG_CDB) ? scmd->l_cdb.cdbp : scmd->cmnd;
+}
+
+/* Helper function for following inline function: scsi_cmnd_set_cdb() */
+u8 *__scsi_cmnd_set_cdb(struct scsi_cmnd *scmd, const u8 *cdb, u16 cdb_len);
+
+/*
+ * If 'cdb' non-NULL then cdb_len bytes starting at cdb are copied into
+ * *scmd . Irrespective of that, the return value points to the start
+ * of a buffer owned by *scmd that has, or can receive, cdb_len bytes
+ * representing a SCSI CDB. If 'cdb' is NULL then the returned buffer
+ * is zero-ed for at least cdb_len bytes. Sets scsi_cmnd::cmd_len .
+ */
+static inline u8 *scsi_cmnd_set_cdb(struct scsi_cmnd *scmd, const u8 *cdb,
+ u16 cdb_len)
+{
+ if ((scmd->flags & SCMD_LONG_CDB) ||
+ cdb_len > SCSI_MAX_COMPILE_TIME_CDB_LEN)
+ return __scsi_cmnd_set_cdb(scmd, cdb, cdb_len);
+ scmd->cmd_len = cdb_len;
+ if (cdb_len > 0) {
+ if (cdb)
+ memcpy(scmd->cmnd, cdb, cdb_len);
+ else
+ memset(scmd->cmnd, 0, cdb_len);
+ }
+ return scmd->cmnd;
+}
+
void scsi_done(struct scsi_cmnd *cmd);
void scsi_done_direct(struct scsi_cmnd *cmd);
@@ -386,7 +465,16 @@ static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
extern void scsi_build_sense(struct scsi_cmnd *scmd, int desc,
u8 key, u8 asc, u8 ascq);
+/* Constructor for request object containing scsi_cmnd sub-object */
struct request *scsi_alloc_request(struct request_queue *q,
unsigned int op, blk_mq_req_flags_t flags);
+/* Destructor for request object via pointer to scsi_cmnd sub-object */
+static inline void scsi_free_cmnd(struct scsi_cmnd *scmd)
+{
+ if (unlikely(scmd->flags & SCMD_LONG_CDB))
+ kfree(scmd->l_cdb.cdbp);
+ blk_mq_free_request(blk_mq_rq_from_pdu(scmd));
+}
+
#endif /* _SCSI_SCSI_CMND_H */
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 1ae08e81339f..3278d3cb5a18 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -34,11 +34,15 @@ struct scsi_eh_save {
int result;
unsigned int resid_len;
int eh_eflags;
+ int flags;
enum dma_data_direction data_direction;
unsigned underflow;
unsigned char cmd_len;
unsigned char prot_op;
- unsigned char cmnd[32];
+ union { /* selected via (scsi_cmnd::flags & SCMD_LONG_CDB) */
+ u8 cmnd[SCSI_MAX_COMPILE_TIME_CDB_LEN]; /* CDB when selector 0 */
+ struct scsi_long_cdb l_cdb; /* when selector != 0 */
+ };
struct scsi_data_buffer sdb;
struct scatterlist sense_sgl;
};
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 3/6] sg: reinstate cmd_len > 32 Douglas Gilbert
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
Although currently unlikely to be needed, allow the sd driver to
issue SCSI commands whose CDB length is > 32. Remove the
direct access to the scsi_cmnd::cmnd field and use an access
function instead. Use 'cdb' rather than 'cmd' to refer to the
SCSI CDB field.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/sd.c | 176 +++++++++++++++++++++++-------------------
drivers/scsi/sd_zbc.c | 12 +--
2 files changed, 101 insertions(+), 87 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a390679cf458..f0fc5789ccbf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -842,6 +842,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
unsigned int data_len = 24;
char *buf;
+ u8 *cdb;
rq->special_vec.bv_page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
if (!rq->special_vec.bv_page)
@@ -851,9 +852,9 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
rq->special_vec.bv_len = data_len;
rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
- cmd->cmd_len = 10;
- cmd->cmnd[0] = UNMAP;
- cmd->cmnd[8] = 24;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 10);
+ cdb[0] = UNMAP;
+ cdb[8] = 24;
buf = bvec_virt(&rq->special_vec);
put_unaligned_be16(6 + 16, &buf[0]);
@@ -877,6 +878,7 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
u64 lba = sectors_to_logical(sdp, blk_rq_pos(rq));
u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
u32 data_len = sdp->sector_size;
+ u8 *cdb;
rq->special_vec.bv_page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
if (!rq->special_vec.bv_page)
@@ -886,12 +888,12 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
rq->special_vec.bv_len = data_len;
rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
- cmd->cmd_len = 16;
- cmd->cmnd[0] = WRITE_SAME_16;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 16);
+ cdb[0] = WRITE_SAME_16;
if (unmap)
- cmd->cmnd[1] = 0x8; /* UNMAP */
- put_unaligned_be64(lba, &cmd->cmnd[2]);
- put_unaligned_be32(nr_blocks, &cmd->cmnd[10]);
+ cdb[1] = 0x8; /* UNMAP */
+ put_unaligned_be64(lba, &cdb[2]);
+ put_unaligned_be32(nr_blocks, &cdb[10]);
cmd->allowed = sdkp->max_retries;
cmd->transfersize = data_len;
@@ -909,6 +911,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
u64 lba = sectors_to_logical(sdp, blk_rq_pos(rq));
u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
u32 data_len = sdp->sector_size;
+ u8 *cdb;
rq->special_vec.bv_page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
if (!rq->special_vec.bv_page)
@@ -918,12 +921,12 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
rq->special_vec.bv_len = data_len;
rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
- cmd->cmd_len = 10;
- cmd->cmnd[0] = WRITE_SAME;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 10);
+ cdb[0] = WRITE_SAME;
if (unmap)
- cmd->cmnd[1] = 0x8; /* UNMAP */
- put_unaligned_be32(lba, &cmd->cmnd[2]);
- put_unaligned_be16(nr_blocks, &cmd->cmnd[7]);
+ cdb[1] = 0x8; /* UNMAP */
+ put_unaligned_be32(lba, &cdb[2]);
+ put_unaligned_be16(nr_blocks, &cdb[7]);
cmd->allowed = sdkp->max_retries;
cmd->transfersize = data_len;
@@ -1024,12 +1027,13 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
{
struct request *rq = scsi_cmd_to_rq(cmd);
struct scsi_disk *sdkp = scsi_disk(rq->q->disk);
+ u8 *cdb;
/* flush requests don't perform I/O, zero the S/G table */
memset(&cmd->sdb, 0, sizeof(cmd->sdb));
- cmd->cmnd[0] = SYNCHRONIZE_CACHE;
- cmd->cmd_len = 10;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 10);
+ cdb[0] = SYNCHRONIZE_CACHE;
cmd->transfersize = 0;
cmd->allowed = sdkp->max_retries;
@@ -1041,14 +1045,16 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write,
sector_t lba, unsigned int nr_blocks,
unsigned char flags)
{
- cmd->cmd_len = SD_EXT_CDB_SIZE;
- cmd->cmnd[0] = VARIABLE_LENGTH_CMD;
- cmd->cmnd[7] = 0x18; /* Additional CDB len */
- cmd->cmnd[9] = write ? WRITE_32 : READ_32;
- cmd->cmnd[10] = flags;
- put_unaligned_be64(lba, &cmd->cmnd[12]);
- put_unaligned_be32(lba, &cmd->cmnd[20]); /* Expected Indirect LBA */
- put_unaligned_be32(nr_blocks, &cmd->cmnd[28]);
+ u8 *cdb;
+
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, SD_EXT_CDB_SIZE);
+ cdb[0] = VARIABLE_LENGTH_CMD;
+ cdb[7] = 0x18; /* Additional CDB len */
+ cdb[9] = write ? WRITE_32 : READ_32;
+ cdb[10] = flags;
+ put_unaligned_be64(lba, &cdb[12]);
+ put_unaligned_be32(lba, &cdb[20]); /* Expected Indirect LBA */
+ put_unaligned_be32(nr_blocks, &cdb[28]);
return BLK_STS_OK;
}
@@ -1057,13 +1063,15 @@ static blk_status_t sd_setup_rw16_cmnd(struct scsi_cmnd *cmd, bool write,
sector_t lba, unsigned int nr_blocks,
unsigned char flags)
{
- cmd->cmd_len = 16;
- cmd->cmnd[0] = write ? WRITE_16 : READ_16;
- cmd->cmnd[1] = flags;
- cmd->cmnd[14] = 0;
- cmd->cmnd[15] = 0;
- put_unaligned_be64(lba, &cmd->cmnd[2]);
- put_unaligned_be32(nr_blocks, &cmd->cmnd[10]);
+ u8 *cdb;
+
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 16);
+ cdb[0] = write ? WRITE_16 : READ_16;
+ cdb[1] = flags;
+ cdb[14] = 0;
+ cdb[15] = 0;
+ put_unaligned_be64(lba, &cdb[2]);
+ put_unaligned_be32(nr_blocks, &cdb[10]);
return BLK_STS_OK;
}
@@ -1072,13 +1080,15 @@ static blk_status_t sd_setup_rw10_cmnd(struct scsi_cmnd *cmd, bool write,
sector_t lba, unsigned int nr_blocks,
unsigned char flags)
{
- cmd->cmd_len = 10;
- cmd->cmnd[0] = write ? WRITE_10 : READ_10;
- cmd->cmnd[1] = flags;
- cmd->cmnd[6] = 0;
- cmd->cmnd[9] = 0;
- put_unaligned_be32(lba, &cmd->cmnd[2]);
- put_unaligned_be16(nr_blocks, &cmd->cmnd[7]);
+ u8 *cdb;
+
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 10);
+ cdb[0] = write ? WRITE_10 : READ_10;
+ cdb[1] = flags;
+ cdb[6] = 0;
+ cdb[9] = 0;
+ put_unaligned_be32(lba, &cdb[2]);
+ put_unaligned_be16(nr_blocks, &cdb[7]);
return BLK_STS_OK;
}
@@ -1087,6 +1097,8 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd *cmd, bool write,
sector_t lba, unsigned int nr_blocks,
unsigned char flags)
{
+ u8 *cdb;
+
/* Avoid that 0 blocks gets translated into 256 blocks. */
if (WARN_ON_ONCE(nr_blocks == 0))
return BLK_STS_IOERR;
@@ -1101,13 +1113,13 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd *cmd, bool write,
return BLK_STS_IOERR;
}
- cmd->cmd_len = 6;
- cmd->cmnd[0] = write ? WRITE_6 : READ_6;
- cmd->cmnd[1] = (lba >> 16) & 0x1f;
- cmd->cmnd[2] = (lba >> 8) & 0xff;
- cmd->cmnd[3] = lba & 0xff;
- cmd->cmnd[4] = nr_blocks;
- cmd->cmnd[5] = 0;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 6);
+ cdb[0] = write ? WRITE_6 : READ_6;
+ cdb[1] = (lba >> 16) & 0x1f;
+ cdb[2] = (lba >> 8) & 0xff;
+ cdb[3] = lba & 0xff;
+ cdb[4] = nr_blocks;
+ cdb[5] = 0;
return BLK_STS_OK;
}
@@ -1589,14 +1601,14 @@ static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr)
sshdr = &my_sshdr;
for (retries = 3; retries > 0; --retries) {
- unsigned char cmd[10] = { 0 };
+ unsigned char cdb[10] = { 0 };
- cmd[0] = SYNCHRONIZE_CACHE;
+ cdb[0] = SYNCHRONIZE_CACHE;
/*
* Leave the rest of the command zero to indicate
* flush everything.
*/
- res = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, sshdr,
+ res = scsi_execute(sdp, cdb, DMA_NONE, NULL, 0, NULL, sshdr,
timeout, sdkp->max_retries, 0, RQF_PM, NULL);
if (res == 0)
break;
@@ -1710,19 +1722,19 @@ static int sd_pr_command(struct block_device *bdev, u8 sa,
struct scsi_device *sdev = sdkp->device;
struct scsi_sense_hdr sshdr;
int result;
- u8 cmd[16] = { 0, };
+ u8 cdb[16] = { 0, };
u8 data[24] = { 0, };
- cmd[0] = PERSISTENT_RESERVE_OUT;
- cmd[1] = sa;
- cmd[2] = type;
- put_unaligned_be32(sizeof(data), &cmd[5]);
+ cdb[0] = PERSISTENT_RESERVE_OUT;
+ cdb[1] = sa;
+ cdb[2] = type;
+ put_unaligned_be32(sizeof(data), &cdb[5]);
put_unaligned_be64(key, &data[0]);
put_unaligned_be64(sa_key, &data[8]);
data[20] = flags;
- result = scsi_execute_req(sdev, cmd, DMA_TO_DEVICE, &data, sizeof(data),
+ result = scsi_execute_req(sdev, cdb, DMA_TO_DEVICE, &data, sizeof(data),
&sshdr, SD_TIMEOUT, sdkp->max_retries, NULL);
if (scsi_status_is_check_condition(result) &&
@@ -1931,6 +1943,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
struct scsi_sense_hdr sshdr;
struct request *req = scsi_cmd_to_rq(SCpnt);
struct scsi_disk *sdkp = scsi_disk(req->q->disk);
+ const u8 *cdb;
int sense_valid = 0;
int sense_deferred = 0;
@@ -1979,6 +1992,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
(!sense_valid || sense_deferred))
goto out;
+ cdb = scsi_cmnd_get_cdb(SCpnt);
switch (sshdr.sense_key) {
case HARDWARE_ERROR:
case MEDIUM_ERROR:
@@ -2006,13 +2020,13 @@ static int sd_done(struct scsi_cmnd *SCpnt)
break;
case 0x20: /* INVALID COMMAND OPCODE */
case 0x24: /* INVALID FIELD IN CDB */
- switch (SCpnt->cmnd[0]) {
+ switch (cdb[0]) {
case UNMAP:
sd_config_discard(sdkp, SD_LBP_DISABLE);
break;
case WRITE_SAME_16:
case WRITE_SAME:
- if (SCpnt->cmnd[1] & 8) { /* UNMAP */
+ if (cdb[1] & 8) { /* UNMAP */
sd_config_discard(sdkp, SD_LBP_DISABLE);
} else {
sdkp->device->no_write_same = 1;
@@ -2044,7 +2058,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)
static void
sd_spinup_disk(struct scsi_disk *sdkp)
{
- unsigned char cmd[10];
+ u8 cdb[10];
unsigned long spintime_expire = 0;
int retries, spintime;
unsigned int the_result;
@@ -2061,10 +2075,10 @@ sd_spinup_disk(struct scsi_disk *sdkp)
do {
bool media_was_present = sdkp->media_present;
- cmd[0] = TEST_UNIT_READY;
- memset((void *) &cmd[1], 0, 9);
+ cdb[0] = TEST_UNIT_READY;
+ memset((void *) &cdb[1], 0, 9);
- the_result = scsi_execute_req(sdkp->device, cmd,
+ the_result = scsi_execute_req(sdkp->device, cdb,
DMA_NONE, NULL, 0,
&sshdr, SD_TIMEOUT,
sdkp->max_retries, NULL);
@@ -2118,13 +2132,13 @@ sd_spinup_disk(struct scsi_disk *sdkp)
*/
if (!spintime) {
sd_printk(KERN_NOTICE, sdkp, "Spinning up disk...");
- cmd[0] = START_STOP;
- cmd[1] = 1; /* Return immediately */
- memset((void *) &cmd[2], 0, 8);
- cmd[4] = 1; /* Start spin cycle */
+ cdb[0] = START_STOP;
+ cdb[1] = 1; /* Return immediately */
+ memset((void *) &cdb[2], 0, 8);
+ cdb[4] = 1; /* Start spin cycle */
if (sdkp->device->start_stop_pwr_cond)
- cmd[4] |= 1 << 4;
- scsi_execute_req(sdkp->device, cmd, DMA_NONE,
+ cdb[4] |= 1 << 4;
+ scsi_execute_req(sdkp->device, cdb, DMA_NONE,
NULL, 0, &sshdr,
SD_TIMEOUT, sdkp->max_retries,
NULL);
@@ -2247,7 +2261,7 @@ static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
unsigned char *buffer)
{
- unsigned char cmd[16];
+ u8 cdb[16];
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
int the_result;
@@ -2260,13 +2274,13 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
return -EINVAL;
do {
- memset(cmd, 0, 16);
- cmd[0] = SERVICE_ACTION_IN_16;
- cmd[1] = SAI_READ_CAPACITY_16;
- cmd[13] = RC16_LEN;
+ memset(cdb, 0, 16);
+ cdb[0] = SERVICE_ACTION_IN_16;
+ cdb[1] = SAI_READ_CAPACITY_16;
+ cdb[13] = RC16_LEN;
memset(buffer, 0, RC16_LEN);
- the_result = scsi_execute_req(sdp, cmd, DMA_FROM_DEVICE,
+ the_result = scsi_execute_req(sdp, cdb, DMA_FROM_DEVICE,
buffer, RC16_LEN, &sshdr,
SD_TIMEOUT, sdkp->max_retries, NULL);
@@ -2338,7 +2352,7 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
unsigned char *buffer)
{
- unsigned char cmd[16];
+ unsigned char cdb[16];
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
int the_result;
@@ -2347,11 +2361,11 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
unsigned sector_size;
do {
- cmd[0] = READ_CAPACITY;
- memset(&cmd[1], 0, 9);
+ cdb[0] = READ_CAPACITY;
+ memset(&cdb[1], 0, 9);
memset(buffer, 0, 8);
- the_result = scsi_execute_req(sdp, cmd, DMA_FROM_DEVICE,
+ the_result = scsi_execute_req(sdp, cdb, DMA_FROM_DEVICE,
buffer, 8, &sshdr,
SD_TIMEOUT, sdkp->max_retries, NULL);
@@ -3546,21 +3560,21 @@ static void scsi_disk_release(struct device *dev)
static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
{
- unsigned char cmd[6] = { START_STOP }; /* START_VALID */
+ u8 cdb[6] = { START_STOP }; /* START_VALID */
struct scsi_sense_hdr sshdr;
struct scsi_device *sdp = sdkp->device;
int res;
if (start)
- cmd[4] |= 1; /* START */
+ cdb[4] |= 1; /* START */
if (sdp->start_stop_pwr_cond)
- cmd[4] |= start ? 1 << 4 : 3 << 4; /* Active or Standby */
+ cdb[4] |= start ? 1 << 4 : 3 << 4; /* Active or Standby */
if (!scsi_device_online(sdp))
return -ENODEV;
- res = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
+ res = scsi_execute(sdp, cdb, DMA_NONE, NULL, 0, NULL, &sshdr,
SD_TIMEOUT, sdkp->max_retries, 0, RQF_PM, NULL);
if (res) {
sd_print_result(sdkp, "Start/Stop Unit failed", res);
@@ -3700,9 +3714,9 @@ static int sd_resume_runtime(struct device *dev)
if (sdp->ignore_media_change) {
/* clear the device's sense data */
- static const u8 cmd[10] = { REQUEST_SENSE };
+ static const u8 cdb[10] = { REQUEST_SENSE };
- if (scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL,
+ if (scsi_execute(sdp, cdb, DMA_NONE, NULL, 0, NULL,
NULL, sdp->request_queue->rq_timeout, 1, 0,
RQF_PM, NULL))
sd_printk(KERN_NOTICE, sdkp,
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 7f466280993b..5567e6af5b49 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -389,6 +389,7 @@ blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
struct request *rq = scsi_cmd_to_rq(cmd);
sector_t sector = blk_rq_pos(rq);
struct scsi_disk *sdkp = scsi_disk(rq->q->disk);
+ u8 *cdb;
sector_t block = sectors_to_logical(sdkp->device, sector);
blk_status_t ret;
@@ -396,14 +397,13 @@ blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
if (ret != BLK_STS_OK)
return ret;
- cmd->cmd_len = 16;
- memset(cmd->cmnd, 0, cmd->cmd_len);
- cmd->cmnd[0] = ZBC_OUT;
- cmd->cmnd[1] = op;
+ cdb = scsi_cmnd_set_cdb(cmd, NULL, 16);
+ cdb[0] = ZBC_OUT;
+ cdb[1] = op;
if (all)
- cmd->cmnd[14] = 0x1;
+ cdb[14] = 0x1;
else
- put_unaligned_be64(block, &cmd->cmnd[2]);
+ put_unaligned_be64(block, &cdb[2]);
rq->timeout = SD_TIMEOUT;
cmd->sc_data_direction = DMA_NONE;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/6] sg: reinstate cmd_len > 32
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 4/6] bsg: allow " Douglas Gilbert
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
Use the changes to include/scsi/scsi_cmnd.h in earlier patch
to use the scsi_cmnd_set_cdb() function to place a SCSI CDB
in the struct scsi_cmnd object.
When free-ing up a struct request, or its attached scsi_cmnd
sub-object, call scsi_free_cmnd() which ensures that if a
long cdb used its own heap allocation, then that heap is freed.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/sg.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index cbffa712b9f3..653c172a6338 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -77,7 +77,7 @@ static int sg_proc_init(void);
* of sg_io_hdr::cmd_len can only represent 255. All SCSI commands greater
* than 16 bytes are "variable length" whose length is a multiple of 4
*/
-#define SG_MAX_CDB_SIZE 252
+#define SG_MAX_CDB_SIZE SCSI_MAX_RUN_TIME_8BIT_CDB_LEN
#define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
@@ -813,7 +813,7 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
}
if (atomic_read(&sdp->detaching)) {
if (srp->bio) {
- blk_mq_free_request(srp->rq);
+ scsi_free_cmnd(blk_mq_rq_to_pdu(srp->rq));
srp->rq = NULL;
}
@@ -1387,7 +1387,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
* blk_rq_unmap_user() can be called from user context.
*/
srp->rq = NULL;
- blk_mq_free_request(rq);
+ scsi_free_cmnd(scmd);
write_lock_irqsave(&sfp->rq_list_lock, iflags);
if (unlikely(srp->orphan)) {
@@ -1753,14 +1753,14 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
return PTR_ERR(rq);
scmd = blk_mq_rq_to_pdu(rq);
- if (hp->cmd_len > sizeof(scmd->cmnd)) {
- blk_mq_free_request(rq);
+ if (unlikely(hp->cmd_len > SCSI_MAX_RUN_TIME_CDB_LEN)) {
+ scsi_free_cmnd(scmd);
return -EINVAL;
}
-
- memcpy(scmd->cmnd, cmd, hp->cmd_len);
- scmd->cmd_len = hp->cmd_len;
-
+ if (unlikely(!scsi_cmnd_set_cdb(scmd, cmd, hp->cmd_len))) {
+ scsi_free_cmnd(scmd);
+ return -ENOMEM;
+ }
srp->rq = rq;
rq->end_io_data = srp;
scmd->allowed = SG_DEFAULT_RETRIES;
@@ -1845,6 +1845,7 @@ sg_finish_rem_req(Sg_request *srp)
Sg_fd *sfp = srp->parentfp;
Sg_scatter_hold *req_schp = &srp->data;
+ struct request *rq = srp->rq;
SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sfp->parentdp,
"sg_finish_rem_req: res_used=%d\n",
@@ -1852,8 +1853,8 @@ sg_finish_rem_req(Sg_request *srp)
if (srp->bio)
ret = blk_rq_unmap_user(srp->bio);
- if (srp->rq)
- blk_mq_free_request(srp->rq);
+ if (rq)
+ scsi_free_cmnd(blk_mq_rq_to_pdu(rq));
if (srp->res_used)
sg_unlink_reserve(sfp, srp);
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/6] bsg: allow cmd_len > 32
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (2 preceding siblings ...)
2022-04-10 17:36 ` [PATCH v2 3/6] sg: reinstate cmd_len > 32 Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 5/6] scsi_debug: reinstate " Douglas Gilbert
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
Since the bsg interface accesses the CDB via scsi_cmnd::cmnd
directly, change that to use the new access functions.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_bsg.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 96ee35256a16..0001a95c6ce1 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -15,6 +15,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
struct scsi_cmnd *scmd;
struct request *rq;
struct bio *bio;
+ u8 *cdb;
int ret;
if (hdr->protocol != BSG_PROTOCOL_SCSI ||
@@ -33,17 +34,24 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
scmd = blk_mq_rq_to_pdu(rq);
scmd->cmd_len = hdr->request_len;
- if (scmd->cmd_len > sizeof(scmd->cmnd)) {
+ if (unlikely(scmd->cmd_len > SCSI_MAX_RUN_TIME_CDB_LEN)) {
ret = -EINVAL;
goto out_put_request;
}
+ cdb = scsi_cmnd_set_cdb(scmd, NULL, scmd->cmd_len);
+ if (unlikely(!cdb)) {
+ ret = -ENOMEM;
+ goto out_put_request;
+ }
- ret = -EFAULT;
- if (copy_from_user(scmd->cmnd, uptr64(hdr->request), scmd->cmd_len))
+ if (unlikely(copy_from_user(cdb, uptr64(hdr->request), scmd->cmd_len))) {
+ ret = -EFAULT;
goto out_put_request;
- ret = -EPERM;
- if (!scsi_cmd_allowed(scmd->cmnd, mode))
+ }
+ if (unlikely(!scsi_cmd_allowed(cdb, mode))) {
+ ret = -EPERM;
goto out_put_request;
+ }
ret = 0;
if (hdr->dout_xfer_len) {
@@ -54,7 +62,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
hdr->din_xfer_len, GFP_KERNEL);
}
- if (ret)
+ if (unlikely(ret))
goto out_put_request;
bio = rq->bio;
@@ -92,7 +100,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
blk_rq_unmap_user(bio);
out_put_request:
- blk_mq_free_request(rq);
+ scsi_free_cmnd(scmd);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/6] scsi_debug: reinstate cmd_len > 32
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (3 preceding siblings ...)
2022-04-10 17:36 ` [PATCH v2 4/6] bsg: allow " Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-10 17:36 ` [PATCH v2 6/6] st,sr,stex: use scsi_cmnd cdb access functions Douglas Gilbert
2022-04-11 5:03 ` [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Christoph Hellwig
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
Example of converting a SCSI low level driver (LLD) to handle
cmd_len > 32 bytes.
When (opts & 1) print out all cdb_s up to 64 bytes in length
(was up to 32 bytes). For testing using sg_raw in user space
via a sg device to send long cdb_s (i.e. > 32 bytes).
Use 'cdb' when referring to a SCSI CDB as 'cmd' gets confused
with other things such as a pointer to a scsi_cmnd object.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/scsi_debug.c | 410 ++++++++++++++++++++------------------
1 file changed, 213 insertions(+), 197 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c607755cce00..d61d039836a0 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -246,7 +246,7 @@ static const char *sdebug_version_date = "20210520";
#define SDEBUG_MAX_PARTS 4
-#define SDEBUG_MAX_CMD_LEN 32
+#define SDEBUG_MAX_CMD_LEN 64
#define SDEB_XA_NOT_IN_USE XA_MARK_1
@@ -1573,12 +1573,12 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
{
unsigned char pq_pdt;
unsigned char *arr;
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u32 alloc_len, n;
int ret;
bool have_wlun, is_disk, is_zbc, is_disk_zbc;
- alloc_len = get_unaligned_be16(cmd + 3);
+ alloc_len = get_unaligned_be16(cdb + 3);
arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_ATOMIC);
if (! arr)
return DID_REQUEUE << 16;
@@ -1593,11 +1593,11 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
else
pq_pdt = (sdebug_ptype & 0x1f);
arr[0] = pq_pdt;
- if (0x2 & cmd[1]) { /* CMDDT bit set */
+ if (0x2 & cdb[1]) { /* CMDDT bit set */
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 1, 1);
kfree(arr);
return check_condition_result;
- } else if (0x1 & cmd[1]) { /* EVPD bit set */
+ } else if (0x1 & cdb[1]) { /* EVPD bit set */
int lu_id_num, port_group_id, target_dev_id;
u32 len;
char lu_id_str[6];
@@ -1612,8 +1612,8 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
target_dev_id = ((host_no + 1) * 2000) +
(devip->target * 1000) - 3;
len = scnprintf(lu_id_str, 6, "%d", lu_id_num);
- if (0 == cmd[2]) { /* supported vital product data pages */
- arr[1] = cmd[2]; /*sanity */
+ if (0 == cdb[2]) { /* supported vital product data pages */
+ arr[1] = cdb[2]; /*sanity */
n = 4;
arr[n++] = 0x0; /* this page */
arr[n++] = 0x80; /* unit serial number */
@@ -1633,24 +1633,24 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
arr[n++] = 0xb6; /* ZB dev. char. */
}
arr[3] = n - 4; /* number of supported VPD pages */
- } else if (0x80 == cmd[2]) { /* unit serial number */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x80 == cdb[2]) { /* unit serial number */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = len;
memcpy(&arr[4], lu_id_str, len);
- } else if (0x83 == cmd[2]) { /* device identification */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x83 == cdb[2]) { /* device identification */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_83(&arr[4], port_group_id,
target_dev_id, lu_id_num,
lu_id_str, len,
&devip->lu_name);
- } else if (0x84 == cmd[2]) { /* Software interface ident. */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x84 == cdb[2]) { /* Software interface ident. */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_84(&arr[4]);
- } else if (0x85 == cmd[2]) { /* Management network addresses */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x85 == cdb[2]) { /* Management network addresses */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_85(&arr[4]);
- } else if (0x86 == cmd[2]) { /* extended inquiry */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x86 == cdb[2]) { /* extended inquiry */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = 0x3c; /* number of following entries */
if (sdebug_dif == T10_PI_TYPE3_PROTECTION)
arr[4] = 0x4; /* SPT: GRD_CHK:1 */
@@ -1659,31 +1659,31 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
else
arr[4] = 0x0; /* no protection stuff */
arr[5] = 0x7; /* head of q, ordered + simple q's */
- } else if (0x87 == cmd[2]) { /* mode page policy */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x87 == cdb[2]) { /* mode page policy */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = 0x8; /* number of following entries */
arr[4] = 0x2; /* disconnect-reconnect mp */
arr[6] = 0x80; /* mlus, shared */
arr[8] = 0x18; /* protocol specific lu */
arr[10] = 0x82; /* mlus, per initiator port */
- } else if (0x88 == cmd[2]) { /* SCSI Ports */
- arr[1] = cmd[2]; /*sanity */
+ } else if (0x88 == cdb[2]) { /* SCSI Ports */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_88(&arr[4], target_dev_id);
- } else if (is_disk_zbc && 0x89 == cmd[2]) { /* ATA info */
- arr[1] = cmd[2]; /*sanity */
+ } else if (is_disk_zbc && 0x89 == cdb[2]) { /* ATA info */
+ arr[1] = cdb[2]; /*sanity */
n = inquiry_vpd_89(&arr[4]);
put_unaligned_be16(n, arr + 2);
- } else if (is_disk_zbc && 0xb0 == cmd[2]) { /* Block limits */
- arr[1] = cmd[2]; /*sanity */
+ } else if (is_disk_zbc && 0xb0 == cdb[2]) { /* Block limits */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_b0(&arr[4]);
- } else if (is_disk_zbc && 0xb1 == cmd[2]) { /* Block char. */
- arr[1] = cmd[2]; /*sanity */
+ } else if (is_disk_zbc && 0xb1 == cdb[2]) { /* Block char. */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_b1(devip, &arr[4]);
- } else if (is_disk && 0xb2 == cmd[2]) { /* LB Prov. */
- arr[1] = cmd[2]; /*sanity */
+ } else if (is_disk && 0xb2 == cdb[2]) { /* LB Prov. */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_b2(&arr[4]);
- } else if (is_zbc && cmd[2] == 0xb6) { /* ZB dev. charact. */
- arr[1] = cmd[2]; /*sanity */
+ } else if (is_zbc && cdb[2] == 0xb6) { /* ZB dev. charact. */
+ arr[1] = cdb[2]; /*sanity */
arr[3] = inquiry_vpd_b6(devip, &arr[4]);
} else {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, -1);
@@ -1740,10 +1740,10 @@ static unsigned char iec_m_pg[] = {0x1c, 0xa, 0x08, 0, 0, 0, 0, 0,
static int resp_requests(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
unsigned char arr[SCSI_SENSE_BUFFERSIZE]; /* assume >= 18 bytes */
- bool dsense = !!(cmd[1] & 1);
- u32 alloc_len = cmd[4];
+ bool dsense = !!(cdb[1] & 1);
+ u32 alloc_len = cdb[4];
u32 len = 18;
int stopped_state = atomic_read(&devip->stopped);
@@ -1793,16 +1793,16 @@ static int resp_requests(struct scsi_cmnd *scp,
static int resp_start_stop(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
{
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
int power_cond, want_stop, stopped_state;
bool changing;
- power_cond = (cmd[4] & 0xf0) >> 4;
+ power_cond = (cdb[4] & 0xf0) >> 4;
if (power_cond) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 4, 7);
return check_condition_result;
}
- want_stop = !(cmd[4] & 1);
+ want_stop = !(cdb[4] & 1);
stopped_state = atomic_read(&devip->stopped);
if (stopped_state == 2) {
ktime_t now_ts = ktime_get_boottime();
@@ -1828,7 +1828,7 @@ static int resp_start_stop(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
changing = (stopped_state != want_stop);
if (changing)
atomic_xchg(&devip->stopped, want_stop);
- if (!changing || (cmd[1] & 0x1)) /* state unchanged or IMMED bit set in cdb */
+ if (!changing || (cdb[1] & 0x1)) /* state unchanged or IMMED bit set in cdb */
return SDEG_RES_IMMED_MASK;
else
return 0;
@@ -1868,11 +1868,11 @@ static int resp_readcap(struct scsi_cmnd *scp,
static int resp_readcap16(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
unsigned char arr[SDEBUG_READCAP16_ARR_SZ];
u32 alloc_len;
- alloc_len = get_unaligned_be32(cmd + 10);
+ alloc_len = get_unaligned_be32(cdb + 10);
/* following just in case virtual_gb changed */
sdebug_capacity = get_sdebug_capacity();
memset(arr, 0, SDEBUG_READCAP16_ARR_SZ);
@@ -1907,14 +1907,14 @@ static int resp_readcap16(struct scsi_cmnd *scp,
static int resp_report_tgtpgs(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
unsigned char *arr;
int host_no = devip->sdbg_host->shost->host_no;
int port_group_a, port_group_b, port_a, port_b;
u32 alen, n, rlen;
int ret;
- alen = get_unaligned_be32(cmd + 6);
+ alen = get_unaligned_be32(cdb + 6);
arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_ATOMIC);
if (! arr)
return DID_REQUEUE << 16;
@@ -1992,13 +1992,13 @@ static int resp_rsup_opcodes(struct scsi_cmnd *scp,
const struct opcode_info_t *oip;
const struct opcode_info_t *r_oip;
u8 *arr;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
- rctd = !!(cmd[2] & 0x80);
- reporting_opts = cmd[2] & 0x7;
- req_opcode = cmd[3];
- req_sa = get_unaligned_be16(cmd + 4);
- alloc_len = get_unaligned_be32(cmd + 6);
+ rctd = !!(cdb[2] & 0x80);
+ reporting_opts = cdb[2] & 0x7;
+ req_opcode = cdb[3];
+ req_sa = get_unaligned_be16(cdb + 4);
+ alloc_len = get_unaligned_be32(cdb + 6);
if (alloc_len < 4 || alloc_len > 0xffff) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 6, -1);
return check_condition_result;
@@ -2138,11 +2138,11 @@ static int resp_rsup_tmfs(struct scsi_cmnd *scp,
bool repd;
u32 alloc_len, len;
u8 arr[16];
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
memset(arr, 0, sizeof(arr));
- repd = !!(cmd[2] & 0x80);
- alloc_len = get_unaligned_be32(cmd + 6);
+ repd = !!(cdb[2] & 0x80);
+ alloc_len = get_unaligned_be32(cdb + 6);
if (alloc_len < 4) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 6, -1);
return check_condition_result;
@@ -2331,22 +2331,22 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
int target = scp->device->id;
unsigned char *ap;
unsigned char arr[SDEBUG_MAX_MSENSE_SZ];
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
bool dbd, llbaa, msense_6, is_disk, is_zbc, bad_pcode;
- dbd = !!(cmd[1] & 0x8); /* disable block descriptors */
- pcontrol = (cmd[2] & 0xc0) >> 6;
- pcode = cmd[2] & 0x3f;
- subpcode = cmd[3];
- msense_6 = (MODE_SENSE == cmd[0]);
- llbaa = msense_6 ? false : !!(cmd[1] & 0x10);
+ dbd = !!(cdb[1] & 0x8); /* disable block descriptors */
+ pcontrol = (cdb[2] & 0xc0) >> 6;
+ pcode = cdb[2] & 0x3f;
+ subpcode = cdb[3];
+ msense_6 = (MODE_SENSE == cdb[0]);
+ llbaa = msense_6 ? false : !!(cdb[1] & 0x10);
is_disk = (sdebug_ptype == TYPE_DISK);
is_zbc = (devip->zmodel != BLK_ZONED_NONE);
if ((is_disk || is_zbc) && !dbd)
bd_len = llbaa ? 16 : 8;
else
bd_len = 0;
- alloc_len = msense_6 ? cmd[4] : get_unaligned_be16(cmd + 7);
+ alloc_len = msense_6 ? cdb[4] : get_unaligned_be16(cdb + 7);
memset(arr, 0, SDEBUG_MAX_MSENSE_SZ);
if (0x3 == pcontrol) { /* Saving values not supported */
mk_sense_buffer(scp, ILLEGAL_REQUEST, SAVING_PARAMS_UNSUP, 0);
@@ -2494,13 +2494,13 @@ static int resp_mode_select(struct scsi_cmnd *scp,
int pf, sp, ps, md_len, bd_len, off, spf, pg_len;
int param_len, res, mpage;
unsigned char arr[SDEBUG_MAX_MSELECT_SZ];
- unsigned char *cmd = scp->cmnd;
- int mselect6 = (MODE_SELECT == cmd[0]);
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
+ int mselect6 = (MODE_SELECT == cdb[0]);
memset(arr, 0, sizeof(arr));
- pf = cmd[1] & 0x10;
- sp = cmd[1] & 0x1;
- param_len = mselect6 ? cmd[4] : get_unaligned_be16(cmd + 7);
+ pf = cdb[1] & 0x10;
+ sp = cdb[1] & 0x1;
+ param_len = mselect6 ? cdb[4] : get_unaligned_be16(cdb + 7);
if ((0 == pf) || sp || (param_len > SDEBUG_MAX_MSELECT_SZ)) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, mselect6 ? 4 : 7, -1);
return check_condition_result;
@@ -2613,18 +2613,18 @@ static int resp_log_sense(struct scsi_cmnd *scp,
int ppc, sp, pcode, subpcode;
u32 alloc_len, len, n;
unsigned char arr[SDEBUG_MAX_LSENSE_SZ];
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
memset(arr, 0, sizeof(arr));
- ppc = cmd[1] & 0x2;
- sp = cmd[1] & 0x1;
+ ppc = cdb[1] & 0x2;
+ sp = cdb[1] & 0x1;
if (ppc || sp) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 1, ppc ? 1 : 0);
return check_condition_result;
}
- pcode = cmd[2] & 0x3f;
- subpcode = cmd[3] & 0xff;
- alloc_len = get_unaligned_be16(cmd + 7);
+ pcode = cdb[2] & 0x3f;
+ subpcode = cdb[3] & 0xff;
+ alloc_len = get_unaligned_be16(cdb + 7);
arr[0] = pcode;
if (0 == subpcode) {
switch (pcode) {
@@ -3132,6 +3132,7 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
scp->device->hostdata, true);
struct t10_pi_tuple *sdt;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
for (i = 0; i < sectors; i++, ei_lba++) {
sector = start_sec + i;
@@ -3147,7 +3148,7 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec,
* which type of error to return. Otherwise we would
* have to iterate over the PI twice.
*/
- if (scp->cmnd[1] >> 5) { /* RDPROTECT */
+ if (cdb[1] >> 5) { /* RDPROTECT */
ret = dif_verify(sdt, lba2fake_store(sip, sector),
sector, ei_lba);
if (ret) {
@@ -3235,56 +3236,56 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
int ret;
u64 lba;
struct sdeb_store_info *sip = devip2sip(devip, true);
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
- switch (cmd[0]) {
+ switch (cdb[0]) {
case READ_16:
ei_lba = 0;
- lba = get_unaligned_be64(cmd + 2);
- num = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ num = get_unaligned_be32(cdb + 10);
check_prot = true;
break;
case READ_10:
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be16(cdb + 7);
check_prot = true;
break;
case READ_6:
ei_lba = 0;
- lba = (u32)cmd[3] | (u32)cmd[2] << 8 |
- (u32)(cmd[1] & 0x1f) << 16;
- num = (0 == cmd[4]) ? 256 : cmd[4];
+ lba = (u32)cdb[3] | (u32)cdb[2] << 8 |
+ (u32)(cdb[1] & 0x1f) << 16;
+ num = (0 == cdb[4]) ? 256 : cdb[4];
check_prot = true;
break;
case READ_12:
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be32(cmd + 6);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be32(cdb + 6);
check_prot = true;
break;
case XDWRITEREAD_10:
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be16(cdb + 7);
check_prot = false;
break;
default: /* assume READ(32) */
- lba = get_unaligned_be64(cmd + 12);
- ei_lba = get_unaligned_be32(cmd + 20);
- num = get_unaligned_be32(cmd + 28);
+ lba = get_unaligned_be64(cdb + 12);
+ ei_lba = get_unaligned_be32(cdb + 20);
+ num = get_unaligned_be32(cdb + 28);
check_prot = false;
break;
}
if (unlikely(have_dif_prot && check_prot)) {
if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
- (cmd[1] & 0xe0)) {
+ (cdb[1] & 0xe0)) {
mk_sense_invalid_opcode(scp);
return check_condition_result;
}
if ((sdebug_dif == T10_PI_TYPE1_PROTECTION ||
sdebug_dif == T10_PI_TYPE3_PROTECTION) &&
- (cmd[1] & 0xe0) == 0)
+ (cdb[1] & 0xe0) == 0)
sdev_printk(KERN_ERR, scp->device, "Unprotected RD "
"to DIF device\n");
}
@@ -3319,7 +3320,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) {
switch (prot_verify_read(scp, lba, num, ei_lba)) {
case 1: /* Guard tag error */
- if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */
+ if (cdb[1] >> 5 != 3) { /* RDPROTECT != 3 */
sdeb_read_unlock(sip);
mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1);
return check_condition_result;
@@ -3330,7 +3331,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
}
break;
case 3: /* Reference tag error */
- if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */
+ if (cdb[1] >> 5 != 3) { /* RDPROTECT != 3 */
sdeb_read_unlock(sip);
mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3);
return check_condition_result;
@@ -3376,6 +3377,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
int ret;
struct t10_pi_tuple *sdt;
void *daddr;
+ const u8 *cdb = scsi_cmnd_get_cdb(SCpnt);
sector_t sector = start_sec;
int ppage_offset;
int dpage_offset;
@@ -3415,7 +3417,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
sdt = piter.addr + ppage_offset;
daddr = diter.addr + dpage_offset;
- if (SCpnt->cmnd[1] >> 5 != 3) { /* WRPROTECT */
+ if (cdb[1] >> 5 != 3) { /* WRPROTECT */
ret = dif_verify(sdt, daddr, sector, ei_lba);
if (ret)
goto out;
@@ -3532,56 +3534,56 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
int ret;
u64 lba;
struct sdeb_store_info *sip = devip2sip(devip, true);
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
- switch (cmd[0]) {
+ switch (cdb[0]) {
case WRITE_16:
ei_lba = 0;
- lba = get_unaligned_be64(cmd + 2);
- num = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ num = get_unaligned_be32(cdb + 10);
check_prot = true;
break;
case WRITE_10:
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be16(cdb + 7);
check_prot = true;
break;
case WRITE_6:
ei_lba = 0;
- lba = (u32)cmd[3] | (u32)cmd[2] << 8 |
- (u32)(cmd[1] & 0x1f) << 16;
- num = (0 == cmd[4]) ? 256 : cmd[4];
+ lba = (u32)cdb[3] | (u32)cdb[2] << 8 |
+ (u32)(cdb[1] & 0x1f) << 16;
+ num = (0 == cdb[4]) ? 256 : cdb[4];
check_prot = true;
break;
case WRITE_12:
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be32(cmd + 6);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be32(cdb + 6);
check_prot = true;
break;
case 0x53: /* XDWRITEREAD(10) */
ei_lba = 0;
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be16(cdb + 7);
check_prot = false;
break;
default: /* assume WRITE(32) */
- lba = get_unaligned_be64(cmd + 12);
- ei_lba = get_unaligned_be32(cmd + 20);
- num = get_unaligned_be32(cmd + 28);
+ lba = get_unaligned_be64(cdb + 12);
+ ei_lba = get_unaligned_be32(cdb + 20);
+ num = get_unaligned_be32(cdb + 28);
check_prot = false;
break;
}
if (unlikely(have_dif_prot && check_prot)) {
if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
- (cmd[1] & 0xe0)) {
+ (cdb[1] & 0xe0)) {
mk_sense_invalid_opcode(scp);
return check_condition_result;
}
if ((sdebug_dif == T10_PI_TYPE1_PROTECTION ||
sdebug_dif == T10_PI_TYPE3_PROTECTION) &&
- (cmd[1] & 0xe0) == 0)
+ (cdb[1] & 0xe0) == 0)
sdev_printk(KERN_ERR, scp->device, "Unprotected WR "
"to DIF device\n");
}
@@ -3601,7 +3603,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
sdeb_write_unlock(sip);
mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1);
return illegal_condition_result;
- } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */
+ } else if (cdb[1] >> 5 != 3) { /* WRPROTECT != 3 */
sdeb_write_unlock(sip);
mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1);
return check_condition_result;
@@ -3612,7 +3614,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
sdeb_write_unlock(sip);
mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 3);
return illegal_condition_result;
- } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */
+ } else if (cdb[1] >> 5 != 3) { /* WRPROTECT != 3 */
sdeb_write_unlock(sip);
mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3);
return check_condition_result;
@@ -3663,7 +3665,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
static int resp_write_scat(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u8 *lrdp = NULL;
u8 *up;
struct sdeb_store_info *sip = devip2sip(devip, true);
@@ -3677,18 +3679,18 @@ static int resp_write_scat(struct scsi_cmnd *scp,
bool is_16;
static const u32 lrd_size = 32; /* + parameter list header size */
- if (cmd[0] == VARIABLE_LENGTH_CMD) {
+ if (cdb[0] == VARIABLE_LENGTH_CMD) {
is_16 = false;
- wrprotect = (cmd[10] >> 5) & 0x7;
- lbdof = get_unaligned_be16(cmd + 12);
- num_lrd = get_unaligned_be16(cmd + 16);
- bt_len = get_unaligned_be32(cmd + 28);
+ wrprotect = (cdb[10] >> 5) & 0x7;
+ lbdof = get_unaligned_be16(cdb + 12);
+ num_lrd = get_unaligned_be16(cdb + 16);
+ bt_len = get_unaligned_be32(cdb + 28);
} else { /* that leaves WRITE SCATTERED(16) */
is_16 = true;
- wrprotect = (cmd[2] >> 5) & 0x7;
- lbdof = get_unaligned_be16(cmd + 4);
- num_lrd = get_unaligned_be16(cmd + 8);
- bt_len = get_unaligned_be32(cmd + 10);
+ wrprotect = (cdb[2] >> 5) & 0x7;
+ lbdof = get_unaligned_be16(cdb + 4);
+ num_lrd = get_unaligned_be16(cdb + 8);
+ bt_len = get_unaligned_be32(cdb + 10);
if (unlikely(have_dif_prot)) {
if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
wrprotect) {
@@ -3887,21 +3889,21 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
static int resp_write_same_10(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u32 lba;
u16 num;
u32 ei_lba = 0;
bool unmap = false;
- if (cmd[1] & 0x8) {
+ if (cdb[1] & 0x8) {
if (sdebug_lbpws10 == 0) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 1, 3);
return check_condition_result;
} else
unmap = true;
}
- lba = get_unaligned_be32(cmd + 2);
- num = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ num = get_unaligned_be16(cdb + 7);
if (num > sdebug_write_same_length) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 7, -1);
return check_condition_result;
@@ -3912,24 +3914,24 @@ static int resp_write_same_10(struct scsi_cmnd *scp,
static int resp_write_same_16(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u64 lba;
u32 num;
u32 ei_lba = 0;
bool unmap = false;
bool ndob = false;
- if (cmd[1] & 0x8) { /* UNMAP */
+ if (cdb[1] & 0x8) { /* UNMAP */
if (sdebug_lbpws == 0) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 1, 3);
return check_condition_result;
} else
unmap = true;
}
- if (cmd[1] & 0x1) /* NDOB (no data-out buffer, assumes zeroes) */
+ if (cdb[1] & 0x1) /* NDOB (no data-out buffer, assumes zeroes) */
ndob = true;
- lba = get_unaligned_be64(cmd + 2);
- num = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ num = get_unaligned_be32(cdb + 10);
if (num > sdebug_write_same_length) {
mk_sense_invalid_fld(scp, SDEB_IN_CDB, 10, -1);
return check_condition_result;
@@ -3943,12 +3945,12 @@ static int resp_write_same_16(struct scsi_cmnd *scp,
static int resp_write_buffer(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct scsi_device *sdp = scp->device;
struct sdebug_dev_info *dp;
u8 mode;
- mode = cmd[1] & 0x1f;
+ mode = cdb[1] & 0x1f;
switch (mode) {
case 0x4: /* download microcode (MC) and activate (ACT) */
/* set UAs on this device only */
@@ -3989,7 +3991,7 @@ static int resp_write_buffer(struct scsi_cmnd *scp,
static int resp_comp_write(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u8 *arr;
struct sdeb_store_info *sip = devip2sip(devip, true);
u64 lba;
@@ -3999,18 +4001,18 @@ static int resp_comp_write(struct scsi_cmnd *scp,
int ret;
int retval = 0;
- lba = get_unaligned_be64(cmd + 2);
- num = cmd[13]; /* 1 to a maximum of 255 logical blocks */
+ lba = get_unaligned_be64(cdb + 2);
+ num = cdb[13]; /* 1 to a maximum of 255 logical blocks */
if (0 == num)
return 0; /* degenerate case, not an error */
if (sdebug_dif == T10_PI_TYPE2_PROTECTION &&
- (cmd[1] & 0xe0)) {
+ (cdb[1] & 0xe0)) {
mk_sense_invalid_opcode(scp);
return check_condition_result;
}
if ((sdebug_dif == T10_PI_TYPE1_PROTECTION ||
sdebug_dif == T10_PI_TYPE3_PROTECTION) &&
- (cmd[1] & 0xe0) == 0)
+ (cdb[1] & 0xe0) == 0)
sdev_printk(KERN_ERR, scp->device, "Unprotected WR "
"to DIF device\n");
ret = check_device_access_params(scp, lba, num, false);
@@ -4057,13 +4059,14 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
{
unsigned char *buf;
struct unmap_block_desc *desc;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_store_info *sip = devip2sip(devip, true);
unsigned int i, payload_len, descriptors;
int ret;
if (!scsi_debug_lbp())
return 0; /* fib and say its done */
- payload_len = get_unaligned_be16(scp->cmnd + 7);
+ payload_len = get_unaligned_be16(cdb + 7);
BUG_ON(scsi_bufflen(scp) != payload_len);
descriptors = (payload_len - 8) / 16;
@@ -4113,14 +4116,14 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
static int resp_get_lba_status(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
u64 lba;
u32 alloc_len, mapped, num;
int ret;
u8 arr[SDEBUG_GET_LBA_STATUS_LEN];
- lba = get_unaligned_be64(cmd + 2);
- alloc_len = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ alloc_len = get_unaligned_be32(cdb + 10);
if (alloc_len < 24)
return 0;
@@ -4158,20 +4161,20 @@ static int resp_sync_cache(struct scsi_cmnd *scp,
int res = 0;
u64 lba;
u32 num_blocks;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
- if (cmd[0] == SYNCHRONIZE_CACHE) { /* 10 byte cdb */
- lba = get_unaligned_be32(cmd + 2);
- num_blocks = get_unaligned_be16(cmd + 7);
+ if (cdb[0] == SYNCHRONIZE_CACHE) { /* 10 byte cdb */
+ lba = get_unaligned_be32(cdb + 2);
+ num_blocks = get_unaligned_be16(cdb + 7);
} else { /* SYNCHRONIZE_CACHE(16) */
- lba = get_unaligned_be64(cmd + 2);
- num_blocks = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ num_blocks = get_unaligned_be32(cdb + 10);
}
if (lba + num_blocks > sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
return check_condition_result;
}
- if (!write_since_sync || (cmd[1] & 0x2))
+ if (!write_since_sync || (cdb[1] & 0x2))
res = SDEG_RES_IMMED_MASK;
else /* delay if write_since_sync and IMMED clear */
write_since_sync = false;
@@ -4192,16 +4195,16 @@ static int resp_pre_fetch(struct scsi_cmnd *scp,
u64 lba;
u64 block, rest = 0;
u32 nblks;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_store_info *sip = devip2sip(devip, true);
u8 *fsp = sip->storep;
- if (cmd[0] == PRE_FETCH) { /* 10 byte cdb */
- lba = get_unaligned_be32(cmd + 2);
- nblks = get_unaligned_be16(cmd + 7);
+ if (cdb[0] == PRE_FETCH) { /* 10 byte cdb */
+ lba = get_unaligned_be32(cdb + 2);
+ nblks = get_unaligned_be16(cdb + 7);
} else { /* PRE-FETCH(16) */
- lba = get_unaligned_be64(cmd + 2);
- nblks = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ nblks = get_unaligned_be32(cdb + 10);
}
if (lba + nblks > sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
@@ -4222,7 +4225,7 @@ static int resp_pre_fetch(struct scsi_cmnd *scp,
prefetch_range(fsp, rest * sdebug_sector_size);
sdeb_read_unlock(sip);
fini:
- if (cmd[1] & 0x2)
+ if (cdb[1] & 0x2)
res = SDEG_RES_IMMED_MASK;
return res | condition_met_result;
}
@@ -4240,7 +4243,7 @@ static int resp_pre_fetch(struct scsi_cmnd *scp,
static int resp_report_luns(struct scsi_cmnd *scp,
struct sdebug_dev_info *devip)
{
- unsigned char *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
unsigned int alloc_len;
unsigned char select_report;
u64 lun;
@@ -4256,8 +4259,8 @@ static int resp_report_luns(struct scsi_cmnd *scp,
clear_luns_changed_on_target(devip);
- select_report = cmd[2];
- alloc_len = get_unaligned_be32(cmd + 6);
+ select_report = cdb[2];
+ alloc_len = get_unaligned_be32(cdb + 6);
if (alloc_len < 4) {
pr_err("alloc len too small %d\n", alloc_len);
@@ -4339,10 +4342,10 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
const u32 lb_size = sdebug_sector_size;
u64 lba;
u8 *arr;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_store_info *sip = devip2sip(devip, true);
- bytchk = (cmd[1] >> 1) & 0x3;
+ bytchk = (cdb[1] >> 1) & 0x3;
if (bytchk == 0) {
return 0; /* always claim internal verify okay */
} else if (bytchk == 2) {
@@ -4351,14 +4354,14 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
} else if (bytchk == 3) {
is_bytchk3 = true; /* 1 block sent, compared repeatedly */
}
- switch (cmd[0]) {
+ switch (cdb[0]) {
case VERIFY_16:
- lba = get_unaligned_be64(cmd + 2);
- vnum = get_unaligned_be32(cmd + 10);
+ lba = get_unaligned_be64(cdb + 2);
+ vnum = get_unaligned_be32(cdb + 10);
break;
case VERIFY: /* is VERIFY(10) */
- lba = get_unaligned_be32(cmd + 2);
- vnum = get_unaligned_be16(cmd + 7);
+ lba = get_unaligned_be32(cdb + 2);
+ vnum = get_unaligned_be16(cdb + 7);
break;
default:
mk_sense_invalid_opcode(scp);
@@ -4418,7 +4421,7 @@ static int resp_report_zones(struct scsi_cmnd *scp,
bool partial;
u64 lba, zs_lba;
u8 *arr = NULL, *desc;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_zone_state *zsp;
struct sdeb_store_info *sip = devip2sip(devip, false);
@@ -4426,12 +4429,12 @@ static int resp_report_zones(struct scsi_cmnd *scp,
mk_sense_invalid_opcode(scp);
return check_condition_result;
}
- zs_lba = get_unaligned_be64(cmd + 2);
- alloc_len = get_unaligned_be32(cmd + 10);
+ zs_lba = get_unaligned_be64(cdb + 2);
+ alloc_len = get_unaligned_be32(cdb + 10);
if (alloc_len == 0)
return 0; /* not an error */
- rep_opts = cmd[14] & 0x3f;
- partial = cmd[14] & 0x80;
+ rep_opts = cdb[14] & 0x3f;
+ partial = cdb[14] & 0x80;
if (zs_lba >= sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
@@ -4559,9 +4562,9 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
int res = 0;
u64 z_id;
enum sdebug_z_cond zc;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_zone_state *zsp;
- bool all = cmd[14] & 0x01;
+ bool all = cdb[14] & 0x01;
struct sdeb_store_info *sip = devip2sip(devip, false);
if (!sdebug_dev_is_zoned(devip)) {
@@ -4586,7 +4589,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
}
/* Open the specified zone */
- z_id = get_unaligned_be64(cmd + 2);
+ z_id = get_unaligned_be64(cdb + 2);
if (z_id >= sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
res = check_condition_result;
@@ -4635,9 +4638,9 @@ static int resp_close_zone(struct scsi_cmnd *scp,
{
int res = 0;
u64 z_id;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
struct sdeb_zone_state *zsp;
- bool all = cmd[14] & 0x01;
+ bool all = cdb[14] & 0x01;
struct sdeb_store_info *sip = devip2sip(devip, false);
if (!sdebug_dev_is_zoned(devip)) {
@@ -4653,7 +4656,7 @@ static int resp_close_zone(struct scsi_cmnd *scp,
}
/* Close specified zone */
- z_id = get_unaligned_be64(cmd + 2);
+ z_id = get_unaligned_be64(cdb + 2);
if (z_id >= sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
res = check_condition_result;
@@ -4708,8 +4711,8 @@ static int resp_finish_zone(struct scsi_cmnd *scp,
struct sdeb_zone_state *zsp;
int res = 0;
u64 z_id;
- u8 *cmd = scp->cmnd;
- bool all = cmd[14] & 0x01;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
+ bool all = cdb[14] & 0x01;
struct sdeb_store_info *sip = devip2sip(devip, false);
if (!sdebug_dev_is_zoned(devip)) {
@@ -4725,7 +4728,7 @@ static int resp_finish_zone(struct scsi_cmnd *scp,
}
/* Finish the specified zone */
- z_id = get_unaligned_be64(cmd + 2);
+ z_id = get_unaligned_be64(cdb + 2);
if (z_id >= sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
res = check_condition_result;
@@ -4788,8 +4791,8 @@ static int resp_rwp_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
struct sdeb_zone_state *zsp;
int res = 0;
u64 z_id;
- u8 *cmd = scp->cmnd;
- bool all = cmd[14] & 0x01;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
+ bool all = cdb[14] & 0x01;
struct sdeb_store_info *sip = devip2sip(devip, false);
if (!sdebug_dev_is_zoned(devip)) {
@@ -4804,7 +4807,7 @@ static int resp_rwp_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
goto fini;
}
- z_id = get_unaligned_be64(cmd + 2);
+ z_id = get_unaligned_be64(cdb + 2);
if (z_id >= sdebug_capacity) {
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
res = check_condition_result;
@@ -5528,7 +5531,7 @@ static bool inject_on_this_cmd(void)
static int process_deflect_incoming(struct scsi_cmnd *scp)
{
- u8 opcode = scp->cmnd[0];
+ u8 opcode = scsi_cmnd_get_cdb(scp)[0];
if (opcode == SYNCHRONIZE_CACHE || opcode == SYNCHRONIZE_CACHE_16)
return 0;
@@ -7436,6 +7439,7 @@ static int resp_not_ready(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
u64 diff_ns = 0;
ktime_t now_ts = ktime_get_boottime();
struct scsi_device *sdp = scp->device;
+ const u8 *cmd = scsi_cmnd_get_cdb(scp);
stopped_state = atomic_read(&devip->stopped);
if (stopped_state == 2) {
@@ -7451,7 +7455,7 @@ static int resp_not_ready(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
if (sdebug_verbose)
sdev_printk(KERN_INFO, sdp,
"%s: Not ready: in process of becoming ready\n", my_name);
- if (scp->cmnd[0] == TEST_UNIT_READY) {
+ if (cmd[0] == TEST_UNIT_READY) {
u64 tur_nanosecs_to_ready = (u64)sdeb_tur_ms_to_ready * 1000000;
if (diff_ns <= tur_nanosecs_to_ready)
@@ -7604,7 +7608,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
const struct opcode_info_t *oip;
const struct opcode_info_t *r_oip;
struct sdebug_dev_info *devip;
- u8 *cmd = scp->cmnd;
+ const u8 *cdb = scsi_cmnd_get_cdb(scp);
int (*r_pfp)(struct scsi_cmnd *, struct sdebug_dev_info *);
int (*pfp)(struct scsi_cmnd *, struct sdebug_dev_info *) = NULL;
int k, na;
@@ -7612,7 +7616,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
u64 lun_index = sdp->lun & 0x3FFF;
u32 flags;
u16 sa;
- u8 opcode = cmd[0];
+ u8 opcode = cdb[0];
bool has_wlun_rl;
bool inject_now;
@@ -7625,20 +7629,32 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
}
if (unlikely(sdebug_verbose &&
!(SDEBUG_OPT_NO_CDB_NOISE & sdebug_opts))) {
- char b[120];
+ char b[200];
int n, len, sb;
len = scp->cmd_len;
sb = (int)sizeof(b);
- if (len > 32)
- strcpy(b, "too long, over 32 bytes");
+ k = 0;
+ if (len > 64)
+ strcpy(b, "too long, over 64 bytes");
else {
- for (k = 0, n = 0; k < len && n < sb; ++k)
+ for (n = 0; k < len && k < 16 && n < sb; ++k)
n += scnprintf(b + n, sb - n, "%02x ",
- (u32)cmd[k]);
+ (u32)cdb[k]);
}
- sdev_printk(KERN_INFO, sdp, "%s: tag=%#x, cmd %s\n", my_name,
+ sdev_printk(KERN_INFO, sdp, "%s: tag=%#x, cmd: %s\n", my_name,
blk_mq_unique_tag(scsi_cmd_to_rq(scp)), b);
+ while (k > 0 && k < len) {
+ for (n = 0; (k < len && n < sb); ++k) {
+ n += scnprintf(b + n, sb - n, "%02x ",
+ (u32)cdb[k]);
+ if (((k + 1) % 16) == 0) {
+ ++k;
+ break;
+ }
+ }
+ sdev_printk(KERN_INFO, sdp, " extra cmd: %s\n", b);
+ }
}
if (unlikely(inject_now && (sdebug_opts & SDEBUG_OPT_HOST_BUSY)))
return SCSI_MLQUEUE_HOST_BUSY;
@@ -7663,9 +7679,9 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
r_oip = oip;
if (FF_SA & r_oip->flags) {
if (F_SA_LOW & oip->flags)
- sa = 0x1f & cmd[1];
+ sa = 0x1f & cdb[1];
else
- sa = get_unaligned_be16(cmd + 8);
+ sa = get_unaligned_be16(cdb + 8);
for (k = 0; k <= na; oip = r_oip->arrp + k++) {
if (opcode == oip->opcode && sa == oip->sa)
break;
@@ -7703,7 +7719,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
int j;
for (k = 1; k < oip->len_mask[0] && k < 16; ++k) {
- rem = ~oip->len_mask[k] & cmd[k];
+ rem = ~oip->len_mask[k] & cdb[k];
if (rem) {
for (j = 7; j >= 0; --j, rem <<= 1) {
if (0x80 & rem)
@@ -7721,7 +7737,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,
if (errsts)
goto check_cond;
}
- if (unlikely(((F_M_ACCESS & flags) || scp->cmnd[0] == TEST_UNIT_READY) &&
+ if (unlikely(((F_M_ACCESS & flags) || cdb[0] == TEST_UNIT_READY) &&
atomic_read(&devip->stopped))) {
errsts = resp_not_ready(scp, devip);
if (errsts)
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 6/6] st,sr,stex: use scsi_cmnd cdb access functions
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (4 preceding siblings ...)
2022-04-10 17:36 ` [PATCH v2 5/6] scsi_debug: reinstate " Douglas Gilbert
@ 2022-04-10 17:36 ` Douglas Gilbert
2022-04-11 5:03 ` [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Christoph Hellwig
6 siblings, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-10 17:36 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
drivers/scsi/sr.c | 40 ++++++++++++++++++++++------------------
drivers/scsi/st.c | 12 +++++++-----
drivers/scsi/stex.c | 22 +++++++++++++---------
3 files changed, 42 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5ba9df334968..1d76fd309501 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -358,7 +358,9 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
int block = 0, this_count, s_size;
struct scsi_cd *cd;
struct request *rq = scsi_cmd_to_rq(SCpnt);
+ u8 *cdb;
blk_status_t ret;
+ static const unsigned int read_write_10_cdb_len = 10;
ret = scsi_alloc_sgtables(SCpnt);
if (ret != BLK_STS_OK)
@@ -390,15 +392,17 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
goto out;
}
+ /* scsi_cmnd_set_cdb() zeros out returned buffer */
+ cdb = scsi_cmnd_set_cdb(SCpnt, NULL, read_write_10_cdb_len);
switch (req_op(rq)) {
case REQ_OP_WRITE:
if (!cd->writeable)
goto out;
- SCpnt->cmnd[0] = WRITE_10;
+ cdb[0] = WRITE_10;
cd->cdi.media_written = 1;
break;
case REQ_OP_READ:
- SCpnt->cmnd[0] = READ_10;
+ cdb[0] = READ_10;
break;
default:
blk_dump_rq_flags(rq, "Unknown sr command");
@@ -439,7 +443,7 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
"writing" : "reading",
this_count, blk_rq_sectors(rq)));
- SCpnt->cmnd[1] = 0;
+ cdb[1] = 0;
block = (unsigned int)blk_rq_pos(rq) / (s_size >> 9);
if (this_count > 0xffff) {
@@ -447,9 +451,8 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
SCpnt->sdb.length = this_count * s_size;
}
- put_unaligned_be32(block, &SCpnt->cmnd[2]);
- SCpnt->cmnd[6] = SCpnt->cmnd[9] = 0;
- put_unaligned_be16(this_count, &SCpnt->cmnd[7]);
+ put_unaligned_be32(block, &cdb[2]);
+ put_unaligned_be16(this_count, &cdb[7]);
/*
* We shouldn't disconnect in the middle of a sector, so with a dumb
@@ -459,7 +462,6 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
SCpnt->transfersize = cd->device->sector_size;
SCpnt->underflow = this_count << 9;
SCpnt->allowed = MAX_RETRIES;
- SCpnt->cmd_len = 10;
/*
* This indicates that the command is ready from our end to be queued.
@@ -929,6 +931,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
struct scsi_cmnd *scmd;
struct request *rq;
struct bio *bio;
+ u8 *cdb;
int ret;
rq = scsi_alloc_request(disk->queue, REQ_OP_DRV_IN, 0);
@@ -940,17 +943,18 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
if (ret)
goto out_put_request;
- scmd->cmnd[0] = GPCMD_READ_CD;
- scmd->cmnd[1] = 1 << 2;
- scmd->cmnd[2] = (lba >> 24) & 0xff;
- scmd->cmnd[3] = (lba >> 16) & 0xff;
- scmd->cmnd[4] = (lba >> 8) & 0xff;
- scmd->cmnd[5] = lba & 0xff;
- scmd->cmnd[6] = (nr >> 16) & 0xff;
- scmd->cmnd[7] = (nr >> 8) & 0xff;
- scmd->cmnd[8] = nr & 0xff;
- scmd->cmnd[9] = 0xf8;
scmd->cmd_len = 12;
+ cdb = scsi_cmnd_set_cdb(scmd, NULL, scmd->cmd_len);
+ cdb[0] = GPCMD_READ_CD;
+ cdb[1] = 1 << 2;
+ cdb[2] = (lba >> 24) & 0xff;
+ cdb[3] = (lba >> 16) & 0xff;
+ cdb[4] = (lba >> 8) & 0xff;
+ cdb[5] = lba & 0xff;
+ cdb[6] = (nr >> 16) & 0xff;
+ cdb[7] = (nr >> 8) & 0xff;
+ cdb[8] = nr & 0xff;
+ cdb[9] = 0xf8;
rq->timeout = 60 * HZ;
bio = rq->bio;
@@ -967,7 +971,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
if (blk_rq_unmap_user(bio))
ret = -EFAULT;
out_put_request:
- blk_mq_free_request(rq);
+ scsi_free_cmnd(scmd);
return ret;
}
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 56a093a90b92..801486c9b411 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -473,10 +473,11 @@ static void st_release_request(struct st_request *streq)
static void st_do_stats(struct scsi_tape *STp, struct request *req)
{
struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
+ const u8 *cdb = scsi_cmnd_get_cdb(scmd);
ktime_t now;
now = ktime_get();
- if (scmd->cmnd[0] == WRITE_6) {
+ if (cdb[0] == WRITE_6) {
now = ktime_sub(now, STp->stats->write_time);
atomic64_add(ktime_to_ns(now), &STp->stats->tot_write_time);
atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
@@ -490,7 +491,7 @@ static void st_do_stats(struct scsi_tape *STp, struct request *req)
} else
atomic64_add(atomic_read(&STp->stats->last_write_size),
&STp->stats->write_byte_cnt);
- } else if (scmd->cmnd[0] == READ_6) {
+ } else if (cdb[0] == READ_6) {
now = ktime_sub(now, STp->stats->read_time);
atomic64_add(ktime_to_ns(now), &STp->stats->tot_read_time);
atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
@@ -531,7 +532,7 @@ static void st_scsi_execute_end(struct request *req, blk_status_t status)
complete(SRpnt->waiting);
blk_rq_unmap_user(tmp);
- blk_mq_free_request(req);
+ scsi_free_cmnd(scmd);
}
static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
@@ -558,7 +559,7 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
err = blk_rq_map_user(req->q, req, mdata, NULL, bufflen,
GFP_KERNEL);
if (err) {
- blk_mq_free_request(req);
+ scsi_free_cmnd(scmd);
return err;
}
}
@@ -576,7 +577,8 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
SRpnt->bio = req->bio;
scmd->cmd_len = COMMAND_SIZE(cmd[0]);
- memcpy(scmd->cmnd, cmd, scmd->cmd_len);
+ /* Don't check for NULL return as extremely unlikely */
+ scsi_cmnd_set_cdb(scmd, cmd, scmd->cmd_len);
req->timeout = timeout;
scmd->allowed = retries;
req->end_io_data = SRpnt;
diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index e6420f2127ce..8f0d28ef2963 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -597,6 +597,7 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd)
struct Scsi_Host *host;
unsigned int id, lun;
struct req_msg *req;
+ const u8 *cdb;
u16 tag;
host = cmd->device->host;
@@ -611,14 +612,15 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd)
if (unlikely(hba->mu_status != MU_STATE_STARTED))
return SCSI_MLQUEUE_HOST_BUSY;
- switch (cmd->cmnd[0]) {
+ cdb = scsi_cmnd_get_cdb(cmd);
+ switch (cdb[0]) {
case MODE_SENSE_10:
{
static char ms10_caching_page[12] =
{ 0, 0x12, 0, 0, 0, 0, 0, 0, 0x8, 0xa, 0x4, 0 };
unsigned char page;
- page = cmd->cmnd[2] & 0x3f;
+ page = cdb[2] & 0x3f;
if (page == 0x8 || page == 0x3f) {
scsi_sg_copy_from_buffer(cmd, ms10_caching_page,
sizeof(ms10_caching_page));
@@ -655,7 +657,7 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd)
if (id != host->max_id - 1)
break;
if (!lun && !cmd->device->channel &&
- (cmd->cmnd[1] & INQUIRY_EVPD) == 0) {
+ (cdb[1] & INQUIRY_EVPD) == 0) {
scsi_sg_copy_from_buffer(cmd, (void *)console_inq_page,
sizeof(console_inq_page));
cmd->result = DID_OK << 16;
@@ -664,7 +666,7 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd)
stex_invalid_field(cmd, done);
return 0;
case PASSTHRU_CMD:
- if (cmd->cmnd[1] == PASSTHRU_GET_DRVVER) {
+ if (cdb[1] == PASSTHRU_GET_DRVVER) {
struct st_drvver ver;
size_t cp_len = sizeof(ver);
@@ -699,7 +701,7 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd)
req->target = id;
/* cdb */
- memcpy(req->cdb, cmd->cmnd, STEX_CDB_LENGTH);
+ memcpy(req->cdb, cdb, STEX_CDB_LENGTH);
if (cmd->sc_data_direction == DMA_FROM_DEVICE)
req->data_dir = MSG_DATA_DIR_IN;
@@ -783,8 +785,8 @@ static void stex_copy_data(struct st_ccb *ccb,
static void stex_check_cmd(struct st_hba *hba,
struct st_ccb *ccb, struct status_msg *resp)
{
- if (ccb->cmd->cmnd[0] == MGT_CMD &&
- resp->scsi_status != SAM_STAT_CHECK_CONDITION)
+ if (scsi_cmnd_get_cdb(ccb->cmd)[0] == MGT_CMD &&
+ resp->scsi_status != SAM_STAT_CHECK_CONDITION)
scsi_set_resid(ccb->cmd, scsi_bufflen(ccb->cmd) -
le32_to_cpu(*(__le32 *)&resp->variable[0]));
}
@@ -858,11 +860,13 @@ static void stex_mu_intr(struct st_hba *hba, u32 doorbell)
ccb->scsi_status = resp->scsi_status;
if (likely(ccb->cmd != NULL)) {
+ const u8 *cdb = scsi_cmnd_get_cdb(ccb->cmd);
+
if (hba->cardtype == st_yosemite)
stex_check_cmd(hba, ccb, resp);
- if (unlikely(ccb->cmd->cmnd[0] == PASSTHRU_CMD &&
- ccb->cmd->cmnd[1] == PASSTHRU_GET_ADAPTER))
+ if (unlikely(cdb[0] == PASSTHRU_CMD &&
+ cdb[1] == PASSTHRU_GET_ADAPTER))
stex_controller_info(hba, ccb);
scsi_dma_unmap(ccb->cmd);
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len
2022-04-10 17:36 [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (5 preceding siblings ...)
2022-04-10 17:36 ` [PATCH v2 6/6] st,sr,stex: use scsi_cmnd cdb access functions Douglas Gilbert
@ 2022-04-11 5:03 ` Christoph Hellwig
2022-04-11 15:06 ` Douglas Gilbert
6 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2022-04-11 5:03 UTC (permalink / raw)
To: Douglas Gilbert; +Cc: linux-scsi, martin.petersen, jejb, hare, bvanassche, hch
This still misses any good explanation of why we want all this.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len
2022-04-11 5:03 ` [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len Christoph Hellwig
@ 2022-04-11 15:06 ` Douglas Gilbert
2022-04-11 15:52 ` Christoph Hellwig
0 siblings, 1 reply; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-11 15:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi, martin.petersen, jejb, hare, bvanassche
On 2022-04-11 01:03, Christoph Hellwig wrote:
> This still misses any good explanation of why we want all this.
Advantages:
- undoes regression in ce70fd9a551af, that is:
- cdb_len > 32 no longer allowed (visible to the user space), undone
- but we still have this one:
- prior to lk5.18 sizeof(scsi_cmnd::cmnd) is that of a
pointer but >= lk5.18 sizeof(scsi_cmnd::cmnd) is 32 (or 16)
- makes all scsi_cmnd objects 16 bytes smaller
- hides the poorly named dtor for scsi_cmnd objects (blk_mq_free_request)
within a more intuitively named inline: scsi_free_cmnd
- scsi_free_cmnd() allows other cleanups to be hooked, like the one
proposed to free the long CDB heap, if used
- supplies three access functions for manipulating CDBs.
scsi_cmnd_set_cdb() removes the need for memset()s and cdb[n]=0 code,
and setting scsi_cmnd::cmd_len when ULDs and LLDs are building CDBs
- allows scsi_cmnd::cmnd to be renamed scsi_cmnd::__cdb in the future
to encourage the use of those access functions
- patches to code accessing scsi_cmnd::cmnd change the name of a SCSI
CDB (a byte array) to 'cdb' rather than the confusing terms: 'cmnd'
or 'cmd'
Disadvantages:
- burdens each access to a CDB with (scsi_cmnd::flags & SCMD_LONG_CDB)
check
- LLDs that want to fetch 32 byte CDBs (or longer) need to use the
scsi_cmnd_get_cdb() access function. For CDB lengths <= 16 bytes
they can continue to access scsi_cmnd::cmnd directly
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len
2022-04-11 15:06 ` Douglas Gilbert
@ 2022-04-11 15:52 ` Christoph Hellwig
2022-04-12 3:05 ` Douglas Gilbert
0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2022-04-11 15:52 UTC (permalink / raw)
To: Douglas Gilbert
Cc: Christoph Hellwig, linux-scsi, martin.petersen, jejb, hare,
bvanassche
On Mon, Apr 11, 2022 at 11:06:17AM -0400, Douglas Gilbert wrote:
> On 2022-04-11 01:03, Christoph Hellwig wrote:
>> This still misses any good explanation of why we want all this.
>
> Advantages:
> - undoes regression in ce70fd9a551af, that is:
> - cdb_len > 32 no longer allowed (visible to the user space), undone
What exact regression causes this for real users and no just people
playing around with scsi_debug?
> - but we still have this one:
> - prior to lk5.18 sizeof(scsi_cmnd::cmnd) is that of a
> pointer but >= lk5.18 sizeof(scsi_cmnd::cmnd) is 32 (or 16)
Please check the total size of struct scsi_cmnd, which is what really
matters.
> - makes all scsi_cmnd objects 16 bytes smaller
Do we have data why this matters?
> - hides the poorly named dtor for scsi_cmnd objects (blk_mq_free_request)
> within a more intuitively named inline: scsi_free_cmnd
I don't think this is in any way poorly named.
> Disadvantages:
> - burdens each access to a CDB with (scsi_cmnd::flags & SCMD_LONG_CDB)
> check
> - LLDs that want to fetch 32 byte CDBs (or longer) need to use the
> scsi_cmnd_get_cdb() access function. For CDB lengths <= 16 bytes
> they can continue to access scsi_cmnd::cmnd directly
It adds back the dynamic allocation for 32-byte CDBs that we got rid of.
It also breaks all LLDS actually using 32-byte CDBS currently as far as
I can tell.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len
2022-04-11 15:52 ` Christoph Hellwig
@ 2022-04-12 3:05 ` Douglas Gilbert
2022-04-19 3:26 ` Martin K. Petersen
0 siblings, 1 reply; 13+ messages in thread
From: Douglas Gilbert @ 2022-04-12 3:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi, martin.petersen, jejb, hare, bvanassche
On 2022-04-11 11:52, Christoph Hellwig wrote:
> On Mon, Apr 11, 2022 at 11:06:17AM -0400, Douglas Gilbert wrote:
>> On 2022-04-11 01:03, Christoph Hellwig wrote:
>>> This still misses any good explanation of why we want all this.
>>
>> Advantages:
>> - undoes regression in ce70fd9a551af, that is:
>> - cdb_len > 32 no longer allowed (visible to the user space), undone
>
> What exact regression causes this for real users and no just people
> playing around with scsi_debug?
Sorry, you are regressing something that has been in place for over
20 years and required by SPC (1 through 5) standards. The onus
should not be on me to prove that regression is not safe. It should
be the other way around (i.e. for you to prove that it is safe).
I admit that working with scsi_debug can be fun, but it seems to me a
few other people have found it a useful tool. Some football advice
might apply here: play the ball, not the man.
>> - but we still have this one:
>> - prior to lk5.18 sizeof(scsi_cmnd::cmnd) is that of a
>> pointer but >= lk5.18 sizeof(scsi_cmnd::cmnd) is 32 (or 16)
>
> Please check the total size of struct scsi_cmnd, which is what really
> matters.
From my laptop (64 bit) where scsi_cmnd1 is the original struct scsi_cmnd:
xtwo70 kernel: sizeof(struct scsi_cmnd)=376, sizeof(struct scsi_cmnd1)=392
So is slightly > 4% (higher on 32 bit machines) insignificant?
Since I have that measurement code in place, try a few other things ....
Changing scsi_cmnd::flags to be a u8 and putting sense_buffer and host_scribble
next to one another (they are pointers) gives:
xtwo70 kernel: sizeof(struct scsi_cmnd)=360, sizeof(struct scsi_cmnd1)=392
Now we are at a 8% reduction.
>
>> - makes all scsi_cmnd objects 16 bytes smaller
>
> Do we have data why this matters?
In commit ce70fd9a551af7424a7dace2a1ba05a7de8eae27 you wrote:
"Now that each scsi_request is backed by a scsi_cmnd, there is no need to
indirect the CDB storage. Change all submitters of SCSI passthrough
requests to store the CDB information directly in the scsi_cmnd, and while
doing so allocate the full 32 bytes that cover all Linux supported SCSI
hosts instead of requiring dynamic allocation for > 16 byte CDBs. On
64-bit systems this does not change the size of the scsi_cmnd at all, while
on 32-bit systems it slightly increases it for now, but that increase will
be made up by the removal of the remaining scsi_request fields."
$ cd drivers/scsi
$ find . -name '*.c' -exec grep "SCSI_MAX_VARLEN_CDB_SIZE" {} \; -print
shost->max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE;
./iscsi_tcp.c
shost->max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE;
./cxgbi/libcxgbi.c
include/scsi/scsi_proto.h:#define SCSI_MAX_VARLEN_CDB_SIZE 260
Two examples that make your "the full 32 bytes that cover all ..." assertion
false.
Also those quoted comments seem to give weight to the argument that
writer believes that the size of scsi_cmnd matters. If so, I agree,
smaller is better.
>> - hides the poorly named dtor for scsi_cmnd objects (blk_mq_free_request)
>> within a more intuitively named inline: scsi_free_cmnd
>
> I don't think this is in any way poorly named.
Seriously?
As well, having a scsi_cmnd destructor opens up the possibility of deferring
kmem_cache_free(scsi_sense_cache, cmd->sense_buffer) from scsi_mq_exit_request()
to that destructor. Then if scsi_cmnd objects are re-used,
scsi_mq_init_request() need only get a cmd->sense_buffer if one has not yet
been allocated. Again, I present no data, but pretty obviously a performance
win.
Another advantage of that patchset:
- in scsi_initialize_rq() the patch initializes CBD to Test Unit Ready
(6 zeros), previously it did a memset(scmd->cmnd, 0, 32), so that is
another small win.
That initialization could be further optimized with:
scmd->l_cdb.dummy_tur = 0; /* clears first 8 zeros */
scmd->cmd_len = SCSI_TEST_UNIT_READY_CDB_LEN;
to bypass memset() completely.
>> Disadvantages:
>> - burdens each access to a CDB with (scsi_cmnd::flags & SCMD_LONG_CDB)
>> check
>> - LLDs that want to fetch 32 byte CDBs (or longer) need to use the
>> scsi_cmnd_get_cdb() access function. For CDB lengths <= 16 bytes
>> they can continue to access scsi_cmnd::cmnd directly
>
> It adds back the dynamic allocation for 32-byte CDBs that we got rid of.
> It also breaks all LLDS actually using 32-byte CDBS currently as far as
> I can tell.
As Bart pointed out, the dynamic allocation for 32-byte CDBs is relatively
rare, more than made up for by the 4% reduction in struct scsi_cmnd's size.
As for the second sentence, if this patchset is accepted, I will find
and fix those. The ones I did check limited cdb_s to 16 bytes.
Doug Gilbert
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len
2022-04-12 3:05 ` Douglas Gilbert
@ 2022-04-19 3:26 ` Martin K. Petersen
0 siblings, 0 replies; 13+ messages in thread
From: Martin K. Petersen @ 2022-04-19 3:26 UTC (permalink / raw)
To: Douglas Gilbert
Cc: Christoph Hellwig, linux-scsi, martin.petersen, jejb, hare,
bvanassche
Douglas,
>> What exact regression causes this for real users and no just people
>> playing around with scsi_debug?
>
> Sorry, you are regressing something that has been in place for over 20
> years and required by SPC (1 through 5) standards. The onus should
> not be on me to prove that regression is not safe. It should be the
> other way around (i.e. for you to prove that it is safe).
Christoph's question is valid: Ignoring obsolete command sets that we no
longer support, what is the real world use case for variable length CDBs
larger than 32 bytes? Which devices require them, and do we want to
support those devices?
That fact that a SCSI spec permits something is not the same as saying
we must support it. "Required by SPC" is news to me.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 13+ messages in thread