| 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 | expand |
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
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
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
I am sorry that this is response has taken too long… Martin suggested adding support for SPACE(16) and you said that you are willing to implement that. I think this is the correct direction. I think that there are still in use old drives that don’t support SPACE(16) and it is good if these are not forgotten. In case you want the interim patch applied to prevent incorrect results with SPACE(6), I support that. Thanks, Kai > On 13. Jan 2021, at 4.24, Patrick Strateman <patrick.strateman@gmail.com> wrote: > > 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> Acked-by: Kai Mäkisara <kai.makisara@kolumbus.fi> > --- > drivers/scsi/st.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > 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); > + } > + } > + > memset(cmd, 0, MAX_COMMAND_SIZE); > switch (cmd_in) { > case MTFSFM:
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); + } + } +
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: