[lvc-project] [PATCH 6.1 v2 3/3] scsi: aic79xx: check for non-NULL scb in ahd_linux_queue_abort_cmd

[Boris Belyavtsev]

On Mon Apr 21, 2025 at 5:08 PM +07, Fedor Pchelkin wrote:
> ВНИМАНИЕ: внешний отправитель! Будьте бдительны, в случае сомнений обратитесь в SOC<mailto:soc at usergate.com>.
>
>
> On Mon, 21. Apr 15:16, Boris Belyavtsev wrote:
> > possible NULL pointer dereference in case hardware returns invalid scb
> > index.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
> > Fixes: 73a254621007 ("[SCSI] aic79xx: update to use scsi_transport_spi")
> > Signed-off-by: Boris Belyavtsev <bbelyavtsev at usergate.com>
> > ---
> >  drivers/scsi/aic7xxx/aic79xx_osm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
> > index 17dfc3c72110..f2ae202d2641 100644
> > --- a/drivers/scsi/aic7xxx/aic79xx_osm.c
> > +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
> > @@ -2265,7 +2265,8 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
>
> Дострою фрагмент:
>
>         /*
>          * At this point, pending_scb is the scb associated with the
>          * passed in command.  That command is currently active on the
>          * bus or is in the disconnected state.
>          */
>         ahd_inb(ahd, SAVED_SCSIID);
>         if (last_phase != P_BUSFREE
>             && SCB_GET_TAG(pending_scb) == active_scbptr) {
>
>                 /*
>                  * We're active on the bus, so assert ATN
>
> >                * and hope that the target responds.
> >                */
> >               pending_scb = ahd_lookup_scb(ahd, active_scbptr);
> > -             pending_scb->flags |= SCB_RECOVERY_SCB|SCB_ABORT;
> > +             if (pending_scb != NULL)
> > +                     pending_scb->flags |= SCB_RECOVERY_SCB|SCB_ABORT;
>
> Ну, то есть этот if-блок покрывает "currently active on the bus". И
> драйвер *подразумевает*, что соответствующий scb будет получен. Иначе
> это свидетельствует о серьёзной ошибке, к обработке которой он не
> приспособлен (нужно с нуля переписывать, грубо говоря).
>
> Лучше какой-нибудь WARN_ON_ONCE и переход к метке out, думаю.
>
> >               ahd_outb(ahd, MSG_OUT, HOST_MSG);
> >               ahd_outb(ahd, SCSISIGO, last_phase|ATNO);
> >               scmd_printk(KERN_INFO, cmd, "Device is active, asserting ATN\n");
> > --
> > 2.43.0

Меняю на
                pending_scb = ahd_lookup_scb(ahd, active_scbptr);
                if (WARN_ON_ONCE(pending_scb == NULL)) {
                        retval = FAILED;
                        goto done;
                }


Также пришло письмо в ответ на патч отправленный в linux-scsi от
"James Bottomley" <James.Bottomley at HansenPartnership.com>

On Mon Apr 21, 2025 at 7:12 PM +07, James Bottomley wrote:
> On Mon, 2025-04-21 at 15:16 +0700, Boris Belyavtsev wrote:
> > Add non-NULL checks for ahd_lookup_scb return value.
> >
> > scb could be NULL if faulty hardware return certain incorrect values
> > to the driver.
>
> It's a general principle that we trust values coming from the card ...
> you are, after all, trusting it with your data.  If there's a fault in
> the way the card is operating, we can work around that, so if you have
> a card which is producing these NULLs, can you provide details so we
> can investigate?
>
> Regards,
>
> James

На самом деле, действительно, получение нулевого scb является
неожиданным и у меня нет такой карты, которая могла быть имитировать
подобнно поведение(возвращение scb равных NULL). Что стоит отвечать в
таких случаях?