On Thu, 2013-12-19 at 14:36 +0100, Hannes Reinecke wrote:
> Instead of using a static buffer for inquiry data we should
> rather use the command-provided buffer and implement proper
> bounds checking when writing to it.
> Inquiry is by no means time-critical ...
> 
> Signed-off-by: Hannes Reinecke <h...@suse.de>
> ---
>  drivers/target/target_core_spc.c     | 391 
> +++++++++++++++++++++--------------
>  include/target/target_core_backend.h |   2 +-
>  2 files changed, 235 insertions(+), 158 deletions(-)
> 

Mmmmm, so this used to be the case once upon a time, and was changed to
the current local buffer copy + possible truncate for simplicities sake.

I still favor the copy to an oversized local buffer vs. adding explicit
size checks to every possible assignment..

How about changing the local buffer to heap memory instead, and bumping
SE_INQUIRY_BUF to 1024 bytes..?

--nab

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index f9889fd..279d260 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -697,11 +697,15 @@ spc_emulate_inquiry(struct se_cmd *cmd)
        struct se_portal_group *tpg = cmd->se_lun->lun_sep->sep_tpg;
        unsigned char *rbuf;
        unsigned char *cdb = cmd->t_task_cdb;
-       unsigned char buf[SE_INQUIRY_BUF];
+       unsigned char *buf;
        sense_reason_t ret;
        int p;
 
-       memset(buf, 0, SE_INQUIRY_BUF);
+       buf = kzalloc(SE_INQUIRY_BUF, GFP_KERNEL);
+       if (!buf) {
+               pr_err("Unable to allocate response buffer for INQUIRY\n");
+               return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+       }
 
        if (dev == tpg->tpg_virt_lun0.lun_se_dev)
                buf[0] = 0x3f; /* Not connected */
@@ -734,9 +738,10 @@ spc_emulate_inquiry(struct se_cmd *cmd)
 out:
        rbuf = transport_kmap_data_sg(cmd);
        if (rbuf) {
-               memcpy(rbuf, buf, min_t(u32, sizeof(buf), cmd->data_length));
+               memcpy(rbuf, buf, min_t(u32, SE_INQUIRY_BUF, cmd->data_length));
                transport_kunmap_data_sg(cmd);
        }
+       kfree(buf);
 
        if (!ret)
                target_complete_cmd(cmd, GOOD);
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 1ba19a4..dd87ab4 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -112,7 +112,7 @@
 /* Queue Algorithm Modifier default for restricted reordering in control mode 
page */
 #define DA_EMULATE_REST_REORD                  0
 
-#define SE_INQUIRY_BUF                         768
+#define SE_INQUIRY_BUF                         1024
 #define SE_MODE_PAGE_BUF                       512
 #define SE_SENSE_BUF                           96

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to