Re: [PATCH] scsi: don't panic host on invalid sgtable count

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

On Sat, 2020-01-25 at 00:16 +0900, Johannes Thumshirn wrote:
> If we have an invalid number of entries mapped an sg table, there's
> no need to panic the host, instead we can spit out a warning in dmesg
> and gracefully return an I/O error.

Can we?  This is an assertion failure which should never happen.  If it
does, it's likely an indicator that a system has gone seriously out of
spec for some reason, like internal compromise, CPU/Memory failure or
something else.

The HA view is that panic is appropriate for conditions that should
never happen because it helps the machine fail fast.

James

> While we're at it fix a trailing whitespace in the comment above.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  drivers/scsi/scsi_lib.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 3e7a45d0daca..9bddf54e3def 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -992,12 +992,15 @@ static blk_status_t scsi_init_sgtable(struct
> request *req,
>  			SCSI_INLINE_SG_CNT)))
>  		return BLK_STS_RESOURCE;
>  
> -	/* 
> +	/*
>  	 * Next, walk the list, and fill in the addresses and sizes
> of
>  	 * each segment.
>  	 */
>  	count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
> -	BUG_ON(count > sdb->table.nents);
> +	if (WARN_ON_ONCE(count > sdb->table.nents)) {
> +		sg_free_table_chained(&sdb->table,
> SCSI_INLINE_SG_CNT);
> +		return BLK_STS_IOERR;
> +	}
>  	sdb->table.nents = count;
>  	sdb->length = blk_rq_payload_bytes(req);
>  	return BLK_STS_OK;