Hello! On 5/22/2018 9:15 PM, Kees Cook wrote:
To avoid introducing problems like those fixed in commit f7068114d45e ("sr: pass down correctly sized SCSI sense buffer"), this creates a macro wrapper for scsi_execute() that verifies the size of the sense buffer similar to what was done for command string sizes in commit 3756f6401c30 ("exec: avoid gcc-8 warning for get_task_comm"). Another solution could be to add another argument to scsi_execute(), but this function already takes a lot of arguments and Jens was not fond of that approach. As there was only a pair of dynamically allocated sense buffers, this also moves those 96 bytes onto the stack to avoid triggering the sizeof() check. Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> --- drivers/scsi/scsi_lib.c | 6 +++--- include/scsi/scsi_device.h | 12 +++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-)
[...]
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 7ae177c8e399..1bb87b6c0ad2 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -426,11 +426,21 @@ extern const char *scsi_device_state_name(enum scsi_device_state); extern int scsi_is_sdev_device(const struct device *); extern int scsi_is_target_device(const struct device *); extern void scsi_sanitize_inquiry_string(unsigned char *s, int len); -extern int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, +extern int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, int data_direction, void *buffer, unsigned bufflen, unsigned char *sense, struct scsi_sense_hdr *sshdr, int timeout, int retries, u64 flags, req_flags_t rq_flags, int *resid); +/* Make sure any sense buffer is the correct size. */ +#define scsi_execute(sdev, cmd, data_direction, buffer, bufflen, sense, \ + sshdr, timeout, retries, flags, rq_flags, resid) \ +({ \ + BUILD_BUG_ON((sense) != NULL && \ + sizeof(sense) != SCSI_SENSE_BUFFERSIZE); \
This would only check the size of the 'sense' pointer, no?
+ __scsi_execute(sdev, cmd, data_direction, buffer, bufflen, \ + sense, sshdr, timeout, retries, flags, rq_flags, \ + resid); \ +}) static inline int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd, int data_direction, void *buffer, unsigned bufflen, struct scsi_sense_hdr *sshdr, int timeout,
MBR, Sergei