Re: [PATCH] scsi: ses: Handle positive SCSI error from ses_recv_diag()

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Mon, 2026-02-23 at 17:03 +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 23, 2026 at 04:44:59PM +0100, Greg Kroah-Hartman wrote:
> > ses_recv_diag() can return a positive value, which also means that
> > an
> > error happened, so do not only test for negative values.
> > 
> > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> > Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_2000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/scsi/ses.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> > index 35101e9b7ba7..128042c734cc 100644
> > --- a/drivers/scsi/ses.c
> > +++ b/drivers/scsi/ses.c
> > @@ -215,7 +215,7 @@ static unsigned char
> > *ses_get_page2_descriptor(struct enclosure_device *edev,
> >  	unsigned char *type_ptr = ses_dev->page1_types;
> >  	unsigned char *desc_ptr = ses_dev->page2 + 8;
> >  
> > -	if (ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev-
> > >page2_len) < 0)
> > +	if (ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev-
> > >page2_len))
> >  		return NULL;
> >  
> >  	for (i = 0; i < ses_dev->page1_num_types; i++, type_ptr +=
> > 4) {
> > -- 
> > 2.53.0
> > 
> 
> Along these lines, any specific reason why the following code in
> ses_enclosure_data_process() doesn't also check the return value of
> ses_recv_diag():
> 
> 	/* re-read page 10 */
> 	if (ses_dev->page10)
> 		ses_recv_diag(sdev, 10, ses_dev->page10, ses_dev-
> >page10_len);
> 	/* Page 7 for the descriptors is optional */
> 	result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
> 	if (result)
> 		goto simple_populate;
> 
> Is that because re-reading always succeeds, or because it can fail
> and we just want to ignore it?  It feels odd that the "optional" read
> command for page 7 is checked by not page 10.  But hey, scsi is odd
> :)

Pretty much.  The re-read should never fail for SCSI reasons (although
there's a remote possibility it could fail for transient reasons) the
scsi_execute will already have tried (and logged errors), so there's no
further error handling ses can add so the best thing it can do is
proceed with stale data.

The reason for the page 7 check is because that will fail if the page
does not exist (which is allowed by spec).

Regards,

James