* [PATCH 0/6] scsi: fix scsi_cmd::cmd_len
@ 2022-04-08 3:56 Douglas Gilbert
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 UTC (permalink / raw)
To: linux-scsi; +Cc: martin.petersen, jejb, hare, bvanassche, hch
As described in the first patch of this set, commit ce70fd9a551af
reduced the maximum size of any SCSI commands (CDB length) to
32 bytes. It was previously larger in struct scsi_request which
has now (lk 5.18.0-rc1) been removed.
Use a slightly different scheme than before to support CDB lengths
greater than 32 bytes. Two access function are added, one for read
access, the other for create/write access to the CDB held inside
a scsi_cmnd object.
A scsi_cmnd object is always paired with a struct request object
that immediately precedes it. To note this pairing the new comments
refer to a scsi_cmnd "sub-object". Prior to this patch the
constructor/destructor naming was obscure:
- scsi_alloc_request() to create a pair
- blk_mq_free_request() to destruct a pair
Add a new destructor function:
scsi_free_cmnd(struct scsi_cmnd *scmd)
to make this a bit clearer. Also scsi_free_cmnd() will free up a
pointer to a long cdb buffer on the heap, if one is present.
These changes have been applied to SCSI mid-level and the upper
level drivers (ULDs, e.g. sd). Only one low level driver (LLD)
has been updated: scsi_debug. The rest of the LLDs can continue
to use scsi_cmnd::cmnd directly _unless_ they wish to support
CDB lengths > 32 bytes; in that case they should use
scsi_cmnd_get_cdb().
This patchset is against lk 5.18.0-rc1 and also applies cleanly
to MKP's 5.19/scsi-queue branch.
Douglas Gilbert (6):
scsi_cmnd: reinstate support for cmd_len > 32
sd, sd_zbc: use scsi_cmnd cdb access functions
sg: reinstate cmd_len > 32
bsg: allow cmd_len > 32
scsi_debug: reinstate cmd_len > 32
st,sr: use scsi_cmnd cdb access functions and dtor
drivers/scsi/scsi_bsg.c | 22 +-
drivers/scsi/scsi_debug.c | 410 +++++++++++++++++++-----------------
drivers/scsi/scsi_debugfs.c | 2 +-
drivers/scsi/scsi_error.c | 76 ++++---
drivers/scsi/scsi_ioctl.c | 21 +-
drivers/scsi/scsi_lib.c | 75 ++++++-
drivers/scsi/scsi_logging.c | 11 +-
drivers/scsi/sd.c | 176 +++++++++-------
drivers/scsi/sd_zbc.c | 12 +-
drivers/scsi/sg.c | 21 +-
drivers/scsi/sr.c | 24 ++-
drivers/scsi/st.c | 12 +-
include/scsi/scsi_cmnd.h | 71 ++++++-
include/scsi/scsi_eh.h | 6 +-
14 files changed, 565 insertions(+), 374 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
2022-04-08 5:16 ` Christoph Hellwig
` (2 more replies)
2022-04-08 3:56 ` [PATCH 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions Douglas Gilbert
` (4 subsequent siblings)
5 siblings, 3 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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. Two 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.
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 | 2 +-
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 | 71 ++++++++++++++++++++++++++++++++--
include/scsi/scsi_eh.h | 6 ++-
7 files changed, 205 insertions(+), 57 deletions(-)
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index 217b70c678c3..d585d32a12b7 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -38,7 +38,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..6220139e4433 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_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..1248bfcba1a0 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 = (u8 *)scsi_cmnd_get_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..17f64d32c59d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -25,13 +25,27 @@ struct Scsi_Host;
* So in effect MAX_COMMAND_SIZE means the maximum size command scsi-ml
* supports without specifying a cmd_len by ULD's
*/
-#define MAX_COMMAND_SIZE 16
+#define MAX_COMMAND_SIZE 16 /* old constant, should be removed */
+
+/* This value is used to size a C array, see below if cdb length > 32 */
+#define SCSI_MAX_COMPILE_TIME_CDB_LEN 32
+
+/* Relatively safe SCSI command length whose CDB is 6 zeros */
+#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)
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 */
@@ -52,8 +66,9 @@ struct scsi_pointer {
#define SCMD_TAGGED (1 << 0)
#define SCMD_INITIALIZED (1 << 1)
#define SCMD_LAST (1 << 2)
+#define SCMD_LONG_CDB (1 << 3)
/* 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
@@ -94,7 +109,17 @@ 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, u8 * cdb, u16 cdb_len), or
+ * const u8 *scsi_cmnd_get_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 +177,37 @@ 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;
+}
+
+/*
+ * 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 the SCSI CDB. If 'cdb' is NULL then the returned buffer
+ * is zero-ed for at least cdb_len bytes.
+ */
+u8 *__scsi_cmnd_set_cdb(struct scsi_cmnd *scmd, const u8 *cdb, u16 cdb_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 +442,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] 12+ messages in thread
* [PATCH 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
2022-04-08 3:56 ` [PATCH 3/6] sg: reinstate cmd_len > 32 Douglas Gilbert
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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] 12+ messages in thread
* [PATCH 3/6] sg: reinstate cmd_len > 32
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
2022-04-08 3:56 ` [PATCH 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
2022-04-08 3:56 ` [PATCH 4/6] bsg: allow " Douglas Gilbert
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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 | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index cbffa712b9f3..96d45550646b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -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] 12+ messages in thread
* [PATCH 4/6] bsg: allow cmd_len > 32
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (2 preceding siblings ...)
2022-04-08 3:56 ` [PATCH 3/6] sg: reinstate cmd_len > 32 Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
2022-04-08 3:56 ` [PATCH 5/6] scsi_debug: reinstate " Douglas Gilbert
2022-04-08 3:56 ` [PATCH 6/6] st,sr: use scsi_cmnd cdb access functions and dtor Douglas Gilbert
5 siblings, 0 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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] 12+ messages in thread
* [PATCH 5/6] scsi_debug: reinstate cmd_len > 32
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (3 preceding siblings ...)
2022-04-08 3:56 ` [PATCH 4/6] bsg: allow " Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
2022-04-08 3:56 ` [PATCH 6/6] st,sr: use scsi_cmnd cdb access functions and dtor Douglas Gilbert
5 siblings, 0 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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] 12+ messages in thread
* [PATCH 6/6] st,sr: use scsi_cmnd cdb access functions and dtor
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
` (4 preceding siblings ...)
2022-04-08 3:56 ` [PATCH 5/6] scsi_debug: reinstate " Douglas Gilbert
@ 2022-04-08 3:56 ` Douglas Gilbert
5 siblings, 0 replies; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-08 3:56 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 | 24 +++++++++++++-----------
drivers/scsi/st.c | 12 +++++++-----
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5ba9df334968..f51c8e1bdb2e 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -929,6 +929,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 +941,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 +969,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;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
@ 2022-04-08 5:16 ` Christoph Hellwig
2022-04-08 7:59 ` kernel test robot
2022-04-08 14:55 ` Bart Van Assche
2 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2022-04-08 5:16 UTC (permalink / raw)
To: Douglas Gilbert; +Cc: linux-scsi, martin.petersen, jejb, hare, bvanassche, hch
On Thu, Apr 07, 2022 at 11:56:46PM -0400, Douglas Gilbert wrote:
> 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.
And who in particular is that?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
2022-04-08 5:16 ` Christoph Hellwig
@ 2022-04-08 7:59 ` kernel test robot
2022-04-08 14:55 ` Bart Van Assche
2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2022-04-08 7:59 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi
Cc: llvm, kbuild-all, martin.petersen, jejb, hare, bvanassche, hch
Hi Douglas,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next v5.18-rc1 next-20220407]
[cannot apply to hch-configfs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Douglas-Gilbert/scsi-fix-scsi_cmd-cmd_len/20220408-121036
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: hexagon-randconfig-r041-20220408 (https://download.01.org/0day-ci/archive/20220408/202204081548.EhdacQg9-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c29a51b3a257908aebc01cd7c4655665db317d66)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/efdf7335424993375502b298131c1d106fc5e6d4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Douglas-Gilbert/scsi-fix-scsi_cmd-cmd_len/20220408-121036
git checkout efdf7335424993375502b298131c1d106fc5e6d4
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/scsi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/scsi_ioctl.c:444:28: warning: result of comparison of constant 260 with expression of type 'unsigned char' is always false [-Wtautological-constant-out-of-range-compare]
if (unlikely(hdr->cmd_len > SCSI_MAX_RUN_TIME_CDB_LEN)) {
~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:48:41: note: expanded from macro 'unlikely'
# define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x)))
^
include/linux/compiler.h:33:34: note: expanded from macro '__branch_check__'
______r = __builtin_expect(!!(x), expect); \
^
>> drivers/scsi/scsi_ioctl.c:444:28: warning: result of comparison of constant 260 with expression of type 'unsigned char' is always false [-Wtautological-constant-out-of-range-compare]
if (unlikely(hdr->cmd_len > SCSI_MAX_RUN_TIME_CDB_LEN)) {
~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:48:68: note: expanded from macro 'unlikely'
# define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x)))
^
include/linux/compiler.h:35:19: note: expanded from macro '__branch_check__'
expect, is_constant); \
^~~~~~~~~~~
2 warnings generated.
vim +444 drivers/scsi/scsi_ioctl.c
407
408 static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
409 {
410 unsigned long start_time;
411 ssize_t ret = 0;
412 int writing = 0;
413 int at_head = 0;
414 struct request *rq;
415 struct scsi_cmnd *scmd;
416 struct bio *bio;
417
418 if (hdr->interface_id != 'S')
419 return -EINVAL;
420
421 if (hdr->dxfer_len > (queue_max_hw_sectors(sdev->request_queue) << 9))
422 return -EIO;
423
424 if (hdr->dxfer_len)
425 switch (hdr->dxfer_direction) {
426 default:
427 return -EINVAL;
428 case SG_DXFER_TO_DEV:
429 writing = 1;
430 break;
431 case SG_DXFER_TO_FROM_DEV:
432 case SG_DXFER_FROM_DEV:
433 break;
434 }
435 if (hdr->flags & SG_FLAG_Q_AT_HEAD)
436 at_head = 1;
437
438 rq = scsi_alloc_request(sdev->request_queue, writing ?
439 REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
440 if (IS_ERR(rq))
441 return PTR_ERR(rq);
442 scmd = blk_mq_rq_to_pdu(rq);
443
> 444 if (unlikely(hdr->cmd_len > SCSI_MAX_RUN_TIME_CDB_LEN)) {
445 ret = -EINVAL;
446 goto out_put_request;
447 }
448
449 ret = scsi_fill_sghdr_rq(sdev, rq, hdr, mode);
450 if (ret < 0)
451 goto out_put_request;
452
453 ret = 0;
454 if (hdr->iovec_count) {
455 struct iov_iter i;
456 struct iovec *iov = NULL;
457
458 ret = import_iovec(rq_data_dir(rq), hdr->dxferp,
459 hdr->iovec_count, 0, &iov, &i);
460 if (ret < 0)
461 goto out_put_request;
462
463 /* SG_IO howto says that the shorter of the two wins */
464 iov_iter_truncate(&i, hdr->dxfer_len);
465
466 ret = blk_rq_map_user_iov(rq->q, rq, NULL, &i, GFP_KERNEL);
467 kfree(iov);
468 } else if (hdr->dxfer_len)
469 ret = blk_rq_map_user(rq->q, rq, NULL, hdr->dxferp,
470 hdr->dxfer_len, GFP_KERNEL);
471
472 if (ret)
473 goto out_put_request;
474
475 bio = rq->bio;
476 scmd->allowed = 0;
477
478 start_time = jiffies;
479
480 blk_execute_rq(rq, at_head);
481
482 hdr->duration = jiffies_to_msecs(jiffies - start_time);
483
484 ret = scsi_complete_sghdr_rq(rq, hdr, bio);
485
486 out_put_request:
487 scsi_free_cmnd(scmd);
488 return ret;
489 }
490
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
2022-04-08 5:16 ` Christoph Hellwig
2022-04-08 7:59 ` kernel test robot
@ 2022-04-08 14:55 ` Bart Van Assche
2022-04-09 4:42 ` Douglas Gilbert
2 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2022-04-08 14:55 UTC (permalink / raw)
To: Douglas Gilbert, linux-scsi; +Cc: martin.petersen, jejb, hare, hch
On 4/7/22 20:56, Douglas Gilbert wrote:
> 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);
Does the cast in the above function call cast away constness? There must
be a better solution than casting away constness.
> -bool scsi_cmd_allowed(unsigned char *cmd, fmode_t mode)
> +bool scsi_cmd_allowed(/* const */ unsigned char *cmd, fmode_t mode)
> {
Why has 'const' been commented out?
> @@ -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 = (u8 *)scsi_cmnd_get_cdb(cmd);
> int rtn = 0;
Casting away constness is ugly. Consider providing two versions of
scsi_cmnd_get_cdb(): one version that accepts a const pointer and
returns a const pointer and another version that accepts a non-const
pointer and returns a non-const pointer. Maybe _Generic() or
__same_type() can be used to combine both versions into a single macro?
> +/* This value is used to size a C array, see below if cdb length > 32 */
> +#define SCSI_MAX_COMPILE_TIME_CDB_LEN 32
Since CDBs longer than 16 bytes are rare, how about using 16 as the
maximum compile-time CDB size?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-08 14:55 ` Bart Van Assche
@ 2022-04-09 4:42 ` Douglas Gilbert
2022-04-09 21:38 ` Bart Van Assche
0 siblings, 1 reply; 12+ messages in thread
From: Douglas Gilbert @ 2022-04-09 4:42 UTC (permalink / raw)
To: Bart Van Assche, linux-scsi; +Cc: martin.petersen, jejb, hare, hch
On 2022-04-08 10:55, Bart Van Assche wrote:
> On 4/7/22 20:56, Douglas Gilbert wrote:
>> 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);
>
> Does the cast in the above function call cast away constness? There must be a
> better solution than casting away constness.
The definition of scsi_send_eh_cmnd() is broken, obviously it doesn't
change the cdb of the passed argument. However I don't want to use
the const incorrectness of the existing code to avoid const in
the code I added. So retrofitting needs casts.
>> -bool scsi_cmd_allowed(unsigned char *cmd, fmode_t mode)
>> +bool scsi_cmd_allowed(/* const */ unsigned char *cmd, fmode_t mode)
>> {
>
> Why has 'const' been commented out?
Because I don't want to change that function's interface, just point out
how it is broken.
>> @@ -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 = (u8 *)scsi_cmnd_get_cdb(cmd);
>> int rtn = 0;
>
> Casting away constness is ugly. Consider providing two versions of
> scsi_cmnd_get_cdb(): one version that accepts a const pointer and returns a
> const pointer and another version that accepts a non-const pointer and returns a
> non-const pointer. Maybe _Generic() or __same_type() can be used to combine both
> versions into a single macro?
Yes, probably a scsi_cmnd_get_changeable_cdb() function which is safe as
long as the new cdb_len <= the existing cdb_len . I think it's the only
occasion I did that due to the cdb[1] overwrite in the lun_in_cdb case
(i.e. SCSI-2 SPI).
>> +/* This value is used to size a C array, see below if cdb length > 32 */
>> +#define SCSI_MAX_COMPILE_TIME_CDB_LEN 32
>
> Since CDBs longer than 16 bytes are rare, how about using 16 as the maximum
> compile-time CDB size?
Well that was the way it was before the surgery performed by Christoph.
If reducing the size of the scsi_cmnd structure by another 16 bytes
is that important, it can be easily done. My "long cdb" side of the
union takes 16 bytes currently (12 on a 32 bit machine).
IMO there should be comments added to scsi_cmnd.h to stress an object
of that type is always preceded (in memory) by a struct request object.
They are created as a pair, and are destroyed (freed, destructed) as
a pair.
Doug Gilbert
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32
2022-04-09 4:42 ` Douglas Gilbert
@ 2022-04-09 21:38 ` Bart Van Assche
0 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2022-04-09 21:38 UTC (permalink / raw)
To: dgilbert, linux-scsi; +Cc: martin.petersen, jejb, hare, hch
On 4/8/22 21:42, Douglas Gilbert wrote:
> On 2022-04-08 10:55, Bart Van Assche wrote:
>> On 4/7/22 20:56, Douglas Gilbert wrote:
>>> +/* This value is used to size a C array, see below if cdb length >
>>> 32 */
>>> +#define SCSI_MAX_COMPILE_TIME_CDB_LEN 32
>>
>> Since CDBs longer than 16 bytes are rare, how about using 16 as the
>> maximum compile-time CDB size?
>
> Well that was the way it was before the surgery performed by Christoph.
> If reducing the size of the scsi_cmnd structure by another 16 bytes
> is that important, it can be easily done. My "long cdb" side of the
> union takes 16 bytes currently (12 on a 32 bit machine).
Some environments (e.g. Android TV booting from UFS) are
memory-constrained. Hence the request to keep struct scsi_cmnd as small
as possible (without sacrificing performance for high-end setups).
> IMO there should be comments added to scsi_cmnd.h to stress an object
> of that type is always preceded (in memory) by a struct request object.
> They are created as a pair, and are destroyed (freed, destructed) as
> a pair.
Although adding such a comment seems fine to me: isn't this something
that applies to all blk-mq drivers?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-04-09 21:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-08 3:56 [PATCH 0/6] scsi: fix scsi_cmd::cmd_len Douglas Gilbert
2022-04-08 3:56 ` [PATCH 1/6] scsi_cmnd: reinstate support for cmd_len > 32 Douglas Gilbert
2022-04-08 5:16 ` Christoph Hellwig
2022-04-08 7:59 ` kernel test robot
2022-04-08 14:55 ` Bart Van Assche
2022-04-09 4:42 ` Douglas Gilbert
2022-04-09 21:38 ` Bart Van Assche
2022-04-08 3:56 ` [PATCH 2/6] sd, sd_zbc: use scsi_cmnd cdb access functions Douglas Gilbert
2022-04-08 3:56 ` [PATCH 3/6] sg: reinstate cmd_len > 32 Douglas Gilbert
2022-04-08 3:56 ` [PATCH 4/6] bsg: allow " Douglas Gilbert
2022-04-08 3:56 ` [PATCH 5/6] scsi_debug: reinstate " Douglas Gilbert
2022-04-08 3:56 ` [PATCH 6/6] st,sr: use scsi_cmnd cdb access functions and dtor Douglas Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.