[bug report] scsi: sg: fix SG_DXFER_FROM_DEV transfers

[bug report] scsi: sg: fix SG_DXFER_FROM_DEV transfers

[Dan Carpenter]

Hello Johannes Thumshirn,

The patch 68c59fcea1f2: "scsi: sg: fix SG_DXFER_FROM_DEV transfers"
from Jul 7, 2017, leads to the following static checker warning:

	drivers/scsi/sg.c:762 sg_is_valid_dxfer()
	warn: unsigned 'hp->dxfer_len' is never less than zero.

drivers/scsi/sg.c
   754  static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
   755  {
   756          switch (hp->dxfer_direction) {
   757          case SG_DXFER_NONE:
   758                  if (hp->dxferp || hp->dxfer_len > 0)
   759                          return false;
   760                  return true;
   761          case SG_DXFER_FROM_DEV:
   762                  if (hp->dxfer_len < 0)
                            ^^^^^^^^^^^^^^^^^
Not possible.

   763                          return false;
   764                  return true;
   765          case SG_DXFER_TO_DEV:
   766          case SG_DXFER_TO_FROM_DEV:
   767                  if (!hp->dxferp || hp->dxfer_len == 0)
   768                          return false;
   769                  return true;
   770          case SG_DXFER_UNKNOWN:
   771                  if ((!hp->dxferp && hp->dxfer_len) ||
   772                      (hp->dxferp && hp->dxfer_len == 0))
   773                          return false;
   774                  return true;
   775          default:
   776                  return false;
   777          }
   778  }

Btw, I was looking up the type of hp->dxfer_len and I noticed that the
documentation for hp->resid is wrong.  It says:

	int resid;                  /* [o] dxfer_len - actual_transferred */

My guess is that resid stands for Response ID but I'm not positive.

regards,
dan carpenter

Re: [bug report] scsi: sg: fix SG_DXFER_FROM_DEV transfers

[Johannes Thumshirn]

On Fri, Jul 14, 2017 at 10:46:03AM +0300, Dan Carpenter wrote:
>    761          case SG_DXFER_FROM_DEV:
>    762                  if (hp->dxfer_len < 0)
>                             ^^^^^^^^^^^^^^^^^
> Not possible.

Yup and thanks to:
647         else
648                 hp->dxfer_direction = (mxsize > 0) ? SG_DXFER_FROM_DEV : SG_DXFER_NONE;
649         hp->dxfer_len = mxsize;

I should've noticed that earlier.
Care to send a patch or want me to do?

> 
> Btw, I was looking up the type of hp->dxfer_len and I noticed that the
> documentation for hp->resid is wrong.  It says:
> 
> 	int resid;                  /* [o] dxfer_len - actual_transferred */
> 
> My guess is that resid stands for Response ID but I'm not positive.

Nope, it's the residual length. It's set by scsi_req::resid_len

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [bug report] scsi: sg: fix SG_DXFER_FROM_DEV transfers

[Dan Carpenter]

On Fri, Jul 14, 2017 at 11:01:11AM +0200, Johannes Thumshirn wrote:
> On Fri, Jul 14, 2017 at 10:46:03AM +0300, Dan Carpenter wrote:
> >    761          case SG_DXFER_FROM_DEV:
> >    762                  if (hp->dxfer_len < 0)
> >                             ^^^^^^^^^^^^^^^^^
> > Not possible.
> 
> Yup and thanks to:
> 647         else
> 648                 hp->dxfer_direction = (mxsize > 0) ? SG_DXFER_FROM_DEV : SG_DXFER_NONE;
> 649         hp->dxfer_len = mxsize;
> 
> I should've noticed that earlier.
> Care to send a patch or want me to do?
> 

Could you send the patch?

regards,
dan carpenter