| Message ID | 20241018101655.4207-1-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: scsi_debug: Fix do_device_access() handling of unexpected SG copy length | expand |
On Fri, Oct 18, 2024 at 10:16:55AM +0000, John Garry wrote: > If the sg_copy_buffer() call returns less than sdebug_sector_size, then we > drop out of the copy loop. However, we still report that we copied the > full expected amount, which is not proper. > > Fix by keeping a running total and return that value. > > Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support") > Reported-by: Colin Ian King <colin.i.king@gmail.com> > Suggested-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: John Garry <john.g.garry@oracle.com> Looks good to me. Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> regards, dan carpenter
On 18/10/2024 11:16, John Garry wrote: > If the sg_copy_buffer() call returns less than sdebug_sector_size, then we > drop out of the copy loop. However, we still report that we copied the > full expected amount, which is not proper. > > Fix by keeping a running total and return that value. > > Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support") > Reported-by: Colin Ian King <colin.i.king@gmail.com> > Suggested-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: John Garry <john.g.garry@oracle.com> > > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > index d95f417e24c0..9be2a6a00530 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -3651,7 +3651,7 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, > enum dma_data_direction dir; > struct scsi_data_buffer *sdb = &scp->sdb; > u8 *fsp; > - int i; > + int i, total = 0; > > /* > * Even though reads are inherently atomic (in this driver), we expect > @@ -3688,18 +3688,16 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, > fsp + (block * sdebug_sector_size), > sdebug_sector_size, sg_skip, do_write); > sdeb_data_sector_unlock(sip, do_write); > - if (ret != sdebug_sector_size) { > - ret += (i * sdebug_sector_size); > + total += ret; > + if (ret != sdebug_sector_size) > break; > - } > sg_skip += sdebug_sector_size; > if (++block >= sdebug_store_sectors) > block = 0; > } > - ret = num * sdebug_sector_size; > sdeb_data_unlock(sip, atomic); > > - return ret; > + return total; > } > > /* Returns number of bytes copied or -1 if error. */ Thank you for fixing this. It looks good to me. Reviewed-by: Colin Ian King <colin.i.king@gmail.com> Colin
On Fri, 18 Oct 2024 10:16:55 +0000, John Garry wrote: > If the sg_copy_buffer() call returns less than sdebug_sector_size, then we > drop out of the copy loop. However, we still report that we copied the > full expected amount, which is not proper. > > Fix by keeping a running total and return that value. > > > [...] Applied to 6.12/scsi-fixes, thanks! [1/1] scsi: scsi_debug: Fix do_device_access() handling of unexpected SG copy length https://git.kernel.org/mkp/scsi/c/d28d17a84560
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index d95f417e24c0..9be2a6a00530 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -3651,7 +3651,7 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, enum dma_data_direction dir; struct scsi_data_buffer *sdb = &scp->sdb; u8 *fsp; - int i; + int i, total = 0; /* * Even though reads are inherently atomic (in this driver), we expect @@ -3688,18 +3688,16 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, fsp + (block * sdebug_sector_size), sdebug_sector_size, sg_skip, do_write); sdeb_data_sector_unlock(sip, do_write); - if (ret != sdebug_sector_size) { - ret += (i * sdebug_sector_size); + total += ret; + if (ret != sdebug_sector_size) break; - } sg_skip += sdebug_sector_size; if (++block >= sdebug_store_sectors) block = 0; } - ret = num * sdebug_sector_size; sdeb_data_unlock(sip, atomic); - return ret; + return total; } /* Returns number of bytes copied or -1 if error. */
If the sg_copy_buffer() call returns less than sdebug_sector_size, then we drop out of the copy loop. However, we still report that we copied the full expected amount, which is not proper. Fix by keeping a running total and return that value. Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support") Reported-by: Colin Ian King <colin.i.king@gmail.com> Suggested-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: John Garry <john.g.garry@oracle.com>