On Fri, 18 Oct 2019, Martin K. Petersen wrote:
>
> (Sorry, zhengbin, you opened a can of worms. This is some of our oldest
> and most arcane code in SCSI)
>
A call to set_host_byte(cmd, x) or set_msg_byte(cmd, x) when x & 0x80 is
set clobbers the most significant bytes in cmd->result.
Avoid this implicit sign extension when shifting bits by using an
unsigned formal parameter.
This is a theoretical bug, found by inspection, so I'm sending an untested
RFC patch.
I didn't try to find possible callers that might pass a negative parameter
and neither did I check whether the clobber might be intentional...
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 76ed5e4acd38..ae814fa68bb8 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -306,17 +306,17 @@ static inline struct scsi_data_buffer *scsi_prot(struct scsi_cmnd *cmd)
#define scsi_for_each_prot_sg(cmd, sg, nseg, __i) \
for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i)
-static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
+static inline void set_msg_byte(struct scsi_cmnd *cmd, unsigned char status)
{
cmd->result = (cmd->result & 0xffff00ff) | (status << 8);
}
-static inline void set_host_byte(struct scsi_cmnd *cmd, char status)
+static inline void set_host_byte(struct scsi_cmnd *cmd, unsigned char status)
{
cmd->result = (cmd->result & 0xff00ffff) | (status << 16);
}
-static inline void set_driver_byte(struct scsi_cmnd *cmd, char status)
+static inline void set_driver_byte(struct scsi_cmnd *cmd, unsigned char status)
{
cmd->result = (cmd->result & 0x00ffffff) | (status << 24);
}
