Re: [PATCH] scsi: aic79xx: Fix NULL pointer dereference in device reset

[Hannes Reinecke <hare@suse.com>]

On 10/21/26 12:15 AM, Andrew Luebke wrote:
> Commit c67e63800446 ("scsi: aic79xx: Do not reference SCSI command when
> resetting device") makes ahd_linux_dev_reset() build its TARGET RESET
> SCB with io_ctx set to NULL, but the function still calls
> ahd_set_residual(reset_scb, 0) a few lines later, which is
> scsi_set_resid() on the NULL command. Every call of the device reset
> handler therefore oopses and kills the SCSI error handler thread; all
> further I/O on that host hangs until the machine is power cycled.
> 
> Hit on an Intel SE7501WV2S board (on-board AIC-7902, i686, v7.2.9) when
> a failing Ultra320 disk made the error handler escalate to a device
> reset. Both disks on that channel then stayed in D state:
> 
>    sd 1:0:0:0: [sda] tag#287 Attempting to queue a TARGET RESET message:
>    BUG: kernel NULL pointer dereference, address: 0000009c
>    #PF: supervisor write access in kernel mode
>    EIP: ahd_linux_dev_reset.cold+0xee/0x26e
>    Call Trace:
>     scsi_eh_ready_devs+0x538/0x8d0
>     scsi_error_handler+0x3a2/0x3b0
>     kthread+0xd2/0x120
>    note: scsi_eh_1[78] exited with irqs disabled
> 
> (0x9c is resid_len in struct scsi_cmnd on i386.)
> 
> Removing that call alone is not enough. When the target accepts the
> reset, ahd_handle_nonpkt_busfree() (or, in packetized mode, the task
> management completion) calls ahd_handle_devreset(), and its
> ahd_abort_scbs() walks the target's pending SCBs - the reset SCB
> included - reading and setting their transaction status and freezing
> them through the wrappers in aic79xx_osm.h, which dereference io_ctx.
> The same walk runs if the reset times out and the error handler
> escalates to a bus reset.
> 
> Commit c7f4c5dec651 ("scsi: aic79xx: Fix up NULL command in ahd_done()")
> taught ahd_done() about the NULL command, but not these wrappers.
> 
> Drop the residual write, and let every wrapper that dereferences io_ctx
> accept an SCB without a command: the transaction status reads as
> CAM_REQ_INPROG, the SCSI status as SAM_STAT_GOOD, the transfer
> direction as DMA_NONE and the residual as 0, while setting a status or
> the residual and freezing the SCB do nothing. The reset SCB then
> completes through ahd_done() as that commit intended. The SCSI status,
> residual and transfer direction wrappers get the same check so that no
> wrapper can be handed the reset SCB and crash; the only path from the
> reset SCB to them that is apparent is ahd_handle_scsi_status(), should
> a packetized LUN RESET end in a status error.
> 
> Tested on the SE7501WV2S with the same change applied to v7.2.11
> (i686): a device reset sent with SG_SCSI_RESET_DEVICE to a healthy
> Seagate ST373453LW logs "Bus Device Reset Message Sent" and "Device
> reset returning 0x2002" (SUCCESS), scsi_eh_1 survives and the disk
> keeps working. That run goes through ahd_handle_devreset() and
> ahd_abort_scbs(), so it exercises the transaction status and freeze
> wrappers on the reset SCB. Not tested: the packetized (IU) LUN RESET
> path and the bus reset escalation. The oops was not reproduced on
> purpose on an unpatched kernel; the crash above came from the failing
> disk.
> 
> Fixes: c67e63800446 ("scsi: aic79xx: Do not reference SCSI command when resetting device")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Andrew Luebke <andrew.luebke@gmail.com>
> ---
> 
> Notes:
>      This analysis, the patch and this changelog were prepared with an LLM coding
>      assistant (Claude Opus 5.5, Anthropic) at my direction; I reviewed them and
>      ran the hardware test described above. Build-tested with W=1 for x86_64 and
>      i386 on mkp/scsi.git for-next (db8b4d2bf2d3): no new warnings. sparse and
>      smatch were not run.
> 
>   drivers/scsi/aic7xxx/aic79xx_osm.c |  1 -
>   drivers/scsi/aic7xxx/aic79xx_osm.h | 19 ++++++++++++++++---
>   2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
> index feb1707fe..4c603ad63 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_osm.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
> @@ -819,7 +819,6 @@ ahd_linux_dev_reset(struct scsi_cmnd *cmd)
>   	reset_scb->io_ctx = NULL;
>   	reset_scb->platform_data->dev = dev;
>   	reset_scb->sg_count = 0;
> -	ahd_set_residual(reset_scb, 0);

You actually make this call safe for a NULL argument right at the end of 
this patch, so please omit this line to get the residual count reset for
the non-NULL case.

>   	ahd_set_sense_residual(reset_scb, 0);
>   	reset_scb->platform_data->xfer_len = 0;
>   	reset_scb->hscb->control = 0;
> diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.h b/drivers/scsi/aic7xxx/aic79xx_osm.h
> index c6eb2b97e..b47aa6c98 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_osm.h
> +++ b/drivers/scsi/aic7xxx/aic79xx_osm.h
> @@ -508,7 +508,8 @@ void ahd_cmd_set_transaction_status(struct scsi_cmnd *cmd, uint32_t status)
>   static inline
>   void ahd_set_transaction_status(struct scb *scb, uint32_t status)
>   {
> -	ahd_cmd_set_transaction_status(scb->io_ctx,status);
> +	if (scb->io_ctx)
> +		ahd_cmd_set_transaction_status(scb->io_ctx, status);

Can't you move this check into ahd_cmd_set_transaction_status()?

>   }
>   
>   static inline
> @@ -521,7 +522,8 @@ void ahd_cmd_set_scsi_status(struct scsi_cmnd *cmd, uint32_t status)
>   static inline
>   void ahd_set_scsi_status(struct scb *scb, uint32_t status)
>   {
> -	ahd_cmd_set_scsi_status(scb->io_ctx, status);
> +	if (scb->io_ctx)
> +		ahd_cmd_set_scsi_status(scb->io_ctx, status);

Same here.

>   }
>   
>   static inline
> @@ -533,6 +535,8 @@ uint32_t ahd_cmd_get_transaction_status(struct scsi_cmnd *cmd)
>   static inline
>   uint32_t ahd_get_transaction_status(struct scb *scb)
>   {
> +	if (!scb->io_ctx)
> +		return CAM_REQ_INPROG;
>   	return (ahd_cmd_get_transaction_status(scb->io_ctx));
>   }
>   
> @@ -545,6 +549,8 @@ uint32_t ahd_cmd_get_scsi_status(struct scsi_cmnd *cmd)
>   static inline
>   uint32_t ahd_get_scsi_status(struct scb *scb)
>   {
> +	if (!scb->io_ctx)
> +		return SAM_STAT_GOOD;
>   	return (ahd_cmd_get_scsi_status(scb->io_ctx));
>   }
>   
> @@ -566,13 +572,16 @@ u_long ahd_get_transfer_length(struct scb *scb)
>   static inline
>   int ahd_get_transfer_dir(struct scb *scb)
>   {
> +	if (!scb->io_ctx)
> +		return DMA_NONE;
>   	return (scb->io_ctx->sc_data_direction);
>   }
>   
>   static inline
>   void ahd_set_residual(struct scb *scb, u_long resid)
>   {
> -	scsi_set_resid(scb->io_ctx, resid);
> +	if (scb->io_ctx)
> +		scsi_set_resid(scb->io_ctx, resid);

And here.

>   }
>   
>   static inline
> @@ -584,6 +593,8 @@ void ahd_set_sense_residual(struct scb *scb, u_long resid)
>   static inline
>   u_long ahd_get_residual(struct scb *scb)
>   {
> +	if (!scb->io_ctx)
> +		return 0;
>   	return scsi_get_resid(scb->io_ctx);
>   }
>   
> @@ -631,6 +642,8 @@ void	ahd_platform_freeze_devq(struct ahd_softc *ahd, struct scb *scb);
>   static inline void
>   ahd_freeze_scb(struct scb *scb)
>   {
> +	if (!scb->io_ctx)
> +		return;
>   	if ((scb->io_ctx->result & (CAM_DEV_QFRZN << 16)) == 0) {
>   		scb->io_ctx->result |= CAM_DEV_QFRZN << 16;
>   		scb->platform_data->dev->qfrozen++;
> 
> base-commit: db8b4d2bf2d3e02b4ff78bbf69d00e11795840de

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.com                               +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