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

[Boris Belyavtsev]

On Mon Apr 21, 2025 at 4:51 PM +07, Fedor Pchelkin wrote:
> Патч по наименованию направлен в 6.1, но в адресатах нет
> stable-мэйнтейнеров. Upstream-мэйнтейнерам на стабильные ветки всё равно.
> Просьба наработать опыт и отладить процесс в lvc-patches at linuxtesting.org

Думаю я не совсем понимаю каким образом необходимо добавлять патчи в
определенные версии ядра. На данный момент, проверок на NULL нет в том
числе и в master. Я так понимаю в первую очередь патч должен попасть в
master и затем должен быть портрован в версии 5.10 и/или 6.1 .


> On Mon, 21. Apr 15:16, Boris Belyavtsev wrote:
> > NULL pointer dereference is possible when compiled with AHD_DEBUG and
>                                                      ^^^^^^^^^^^^^^^^^^
> > AHD_SHOW_RECOVERY is set if data in SCBPTR и SCBPTR+1 ports is
>   ^^^^^^^^^^^^^^^^^
> Не только. Ещё раз, после ifdef AHD_DEBUG идёт рабочий фрагмент
>
>         /*
>          * Set this and it will take effect when the
>          * target does a command complete.
>          */
>         ahd_freeze_devq(ahd, scb);
>         ahd_set_transaction_status(scb, CAM_DATA_RUN_ERR);
>         ahd_freeze_scb(scb);
>
> в котором будет наблюдаться разыменовывание NULL и без дебага.
>
> Дополнительный комментарий см. ниже.
>
> > incorrect.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Boris Belyavtsev <bbelyavtsev at usergate.com>
> > ---
> >  drivers/scsi/aic7xxx/aic79xx_core.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c b/drivers/scsi/aic7xxx/aic79xx_core.c
> > index f9372a81cd4e..a4d5376123d3 100644
> > --- a/drivers/scsi/aic7xxx/aic79xx_core.c
> > +++ b/drivers/scsi/aic7xxx/aic79xx_core.c
> > @@ -2205,13 +2205,16 @@ ahd_handle_seqint(struct ahd_softc *ahd, u_int intstat)
> >                       ahd_print_path(ahd, scb);
> >                       printk("data overrun detected %s.  Tag == 0x%x.\n",
> >                              ahd_lookup_phase_entry(lastphase)->phasemsg,
> > -                            SCB_GET_TAG(scb));
> > +                            scb != NULL ? SCB_GET_TAG(scb) : 0);
> >                       ahd_print_path(ahd, scb);
> >                       printk("%s seen Data Phase.  Length = %ld.  "
> >                              "NumSGs = %d.\n",
> >                              ahd_inb(ahd, SEQ_FLAGS) & DPHASE
> >                              ? "Have" : "Haven't",
> > -                            ahd_get_transfer_length(scb), scb->sg_count);
> > +                            scb != NULL ? ahd_get_transfer_length(scb) : -1,
> > +                            scb != NULL ? scb->sg_count : -1);
> > +                     if (scb == NULL)
> > +                             break;
>
> scb == NULL на данном этапе - абсолютно неожидаемое драйвером поведение
> от hardware/firmware. Код sequencer'a в исходниках ядра имеется.
>
> [drivers/scsi/aic7xxx/aic79xx.reg]
>         DATA_OVERRUN,                   /*
>                                          * Target attempted to write
>                                          * beyond the bounds of its
>                                          * command.
>                                          */
>
> https://elixir.bootlin.com/linux/v6.14.3/source/drivers/scsi/aic7xxx/aic79xx.seq#L1671
>
> Предлагаю после ahd_lookup_scb() в случае нулевого scb поставить
> WARN_ON_ONCE и сделать break.

Да, согласен. Изменю патч, чтобы остался лишь break после
ahd_lookup_scb().

if (WARN_ON_ONCE(scb == NULL))
	break;

Вот так будет нормально?