st: reject MTIOCTOP mt_count values out of range for the SPACE(6) scsi command

Message ID CA+-=Uwah2MS_aD+PeSBkQa_=1wCD+3=g6W4PvLnbJ_-px8G97g@mail.gmail.com
State New
Headers show
Series
  • st: reject MTIOCTOP mt_count values out of range for the SPACE(6) scsi command
Related show

Commit Message

Patrick Strateman Jan. 13, 2021, 2:24 a.m.
Values greater than 0x7FFFFF do not fit in the 24 bit big endian two's
complement integer for the underlying scsi SPACE(6) command.

Signed-off-by: Patrick Strateman <patrick.strateman@gmail.com>
---
 drivers/scsi/st.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

     memset(cmd, 0, MAX_COMMAND_SIZE);
     switch (cmd_in) {
     case MTFSFM:

Comments

Martin K. Petersen Jan. 27, 2021, 4:24 a.m. | #1
Patrick,

> Values greater than 0x7FFFFF do not fit in the 24 bit big endian two's

> complement integer for the underlying scsi SPACE(6) command.


I was hoping Kai would chime in.

However, since SPACE(6) has been deprecated for a while, I am not so
keen on blindly enforcing this in the ioctl interface. I would rather
see SPACE(16) support added and then have the supplied count value be
validated based on whether the 6 or 16 byte command is being issued to
the device.

-- 
Martin K. Petersen	Oracle Linux Engineering
Patrick Strateman Jan. 27, 2021, 4:44 a.m. | #2
I'm happy to implement SPACE(16) instead.

I believe the opcode for SPACE(16) is 0x91, but in
include/scsi/scsi_proto.h that value is listed as
SYNCHRONIZE_CACHE_16.

I couldn't figure out which was wrong, so I proposed this to at least
prevent the interface from doing the wrong thing.

On Tue, Jan 26, 2021 at 11:26 PM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>

>

> Patrick,

>

> > Values greater than 0x7FFFFF do not fit in the 24 bit big endian two's

> > complement integer for the underlying scsi SPACE(6) command.

>

> I was hoping Kai would chime in.

>

> However, since SPACE(6) has been deprecated for a while, I am not so

> keen on blindly enforcing this in the ioctl interface. I would rather

> see SPACE(16) support added and then have the supplied count value be

> validated based on whether the 6 or 16 byte command is being issued to

> the device.

>

> --

> Martin K. Petersen      Oracle Linux Engineering
Martin K. Petersen Jan. 27, 2021, 5 a.m. | #3
Patrick,

> I believe the opcode for SPACE(16) is 0x91,


That is correct.

> but in include/scsi/scsi_proto.h that value is listed as

> SYNCHRONIZE_CACHE_16.


Yeah, I'm afraid things have a tendency to be SPC/SBC centric. Feel free
to add SPACE(16) and make a comment about the SSC vs. SBC distinction.

-- 
Martin K. Petersen	Oracle Linux Engineering

Patch

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 43f7624508a9..190fa678d149 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2719,6 +2719,22 @@  static int st_int_ioctl(struct scsi_tape *STp,
unsigned int cmd_in, unsigned lon
     blkno = STps->drv_block;
     at_sm = STps->at_sm;

+    switch (cmd_in) {
+    case MTFSFM:
+    case MTFSF:
+    case MTBSFM:
+    case MTBSF:
+    case MTFSR:
+    case MTBSR:
+    case MTFSS:
+    case MTBSS:
+        // count field for SPACE(6) is a 24 bit big endian two's
complement integer
+        if (arg > 0x7FFFFF) {
+            st_printk(ST_DEB_MSG, STp, "Cannot space more than
0x7FFFFF units.\n");
+            return (-EINVAL);
+        }
+    }
+