From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754470AbeEWIZz (ORCPT ); Wed, 23 May 2018 04:25:55 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:38056 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754319AbeEWIZt (ORCPT ); Wed, 23 May 2018 04:25:49 -0400 X-Google-Smtp-Source: AB8JxZr6DcVQeWkyGL4Q03+p7nVQooCkocBcNzKbj2g7dMIbfxFzj4H7ROKwqlE1NgaiJkgZkzVJdQ== Subject: Re: [PATCH 6/6] scsi: Check sense buffer size at build time To: Kees Cook , Jens Axboe Cc: "Martin K. Petersen" , James Bottomley , Tejun Heo , Borislav Petkov , "David S. Miller" , "Manoj N. Kumar" , "Matthew R. Ochs" , Uma Krishnan , linux-block@vger.kernel.org, linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org References: <20180522181512.39316-1-keescook@chromium.org> <20180522181512.39316-7-keescook@chromium.org> From: Sergei Shtylyov Message-ID: <7de525d4-52f4-80f2-1f55-a3a5c37d7bf9@cogentembedded.com> Date: Wed, 23 May 2018 11:25:45 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180522181512.39316-7-keescook@chromium.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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