| Message ID | 20191021095322.137969-20-hare@suse.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: Revamp result values | expand |
On 10/21/19 2:53 AM, Hannes Reinecke wrote: > When failing to map the user buffer we should return the actual > error code, avoiding the usage of DRIVER_ERROR. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > block/scsi_ioctl.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c > index f5e0ad65e86a..1ab1b8d9641c 100644 > --- a/block/scsi_ioctl.c > +++ b/block/scsi_ioctl.c > @@ -485,9 +485,10 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode, > break; > } > > - if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO)) { > - err = DRIVER_ERROR << 24; > - goto error; > + if (bytes) { > + err = blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO); > + if (err) > + goto error; > } > > blk_execute_rq(q, disk, rq, 0); Since sg_scsi_ioctl() is used to implement SCSI_IOCTL_SEND_COMMAND, does this patch change the ABI between user space and kernel in a backwards-incompatible way? Thanks, Bart.
On 10/21/19 6:44 PM, Bart Van Assche wrote: > On 10/21/19 2:53 AM, Hannes Reinecke wrote: >> When failing to map the user buffer we should return the actual >> error code, avoiding the usage of DRIVER_ERROR. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> block/scsi_ioctl.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c >> index f5e0ad65e86a..1ab1b8d9641c 100644 >> --- a/block/scsi_ioctl.c >> +++ b/block/scsi_ioctl.c >> @@ -485,9 +485,10 @@ int sg_scsi_ioctl(struct request_queue *q, struct >> gendisk *disk, fmode_t mode, >> break; >> } >> - if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO)) { >> - err = DRIVER_ERROR << 24; >> - goto error; >> + if (bytes) { >> + err = blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO); >> + if (err) >> + goto error; >> } >> blk_execute_rq(q, disk, rq, 0); > > Since sg_scsi_ioctl() is used to implement SCSI_IOCTL_SEND_COMMAND, does > this patch change the ABI between user space and kernel in a > backwards-incompatible way? > Not really. We do change the return code, but sg_scsi_ioctl() already returns negative POSIX error numbers on failure. And, in fact, this position is the _only_ possible case where we return something else than either an errno or a SCSI status. Which arguably is an error even in the current code. Cheers, Hannes
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index f5e0ad65e86a..1ab1b8d9641c 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -485,9 +485,10 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode, break; } - if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO)) { - err = DRIVER_ERROR << 24; - goto error; + if (bytes) { + err = blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO); + if (err) + goto error; } blk_execute_rq(q, disk, rq, 0);
When failing to map the user buffer we should return the actual error code, avoiding the usage of DRIVER_ERROR. Signed-off-by: Hannes Reinecke <hare@suse.de> --- block/scsi_ioctl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)