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

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

[Greg Kroah-Hartman]

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

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

[Greg Kroah-Hartman]

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 :)

thank,s

greg k-h

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

[James Bottomley]

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

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

[Hannes Reinecke]

On 2/23/26 16:44, 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) {

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Martin K. Petersen]

On Mon, 23 Feb 2026 16:44:59 +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.
> 
> 

Applied to 7.0/scsi-fixes, thanks!

[1/1] scsi: ses: Handle positive SCSI error from ses_recv_diag()
      https://git.kernel.org/mkp/scsi/c/7a9f448d4412

-- 
Martin K. Petersen