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

[Fedor Pchelkin]

On Fri, 25. Apr 09:52, Boris Belyavtsev wrote:
> 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 .

Да. В стабильные ветки межд. сообщества попадают только патчи, имеющиеся
в master ветке Торвальдса.
https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst

Постановка 6.1 в тему письма сделана поспешно.

> > 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;
> 
> Вот так будет нормально?

Да. Большего мы здесь сделать не можем, ну и драйвер в принципе не
предполагает наличия такой ситуации в этом месте.

Для патчей из текущей серии ситуация следующая: затрагиваются случаи
отсутствия проверки результата функции, зависящие от контекста
взаимодействия с аппаратурой.

Логика отсутствия проверки ясна в достаточной степени. По протоколу
взаимодействия с контроллером и его прошивкой на рассматриваемых этапах
буфер SCSI-команды с заданным индексом должен существовать, и
ahd_lookup_scb() вернёт его. Соответствующие фрагменты документации и
фрагментов кода sequencer'а я приводил в прошлом письме.


Драйвер написан давно. Да, в последнее время наблюдается тенденция, что
драйверы пытаются писать так, чтобы подобных сомнительных ситуаций при
работе с оборудованием было меньше. К патчам на добавление проверок в
сообществе относятся ни шатко ни валко, но стараются всё же необоснованных
лишних проверок в такие места не вставлять.

Джеймс как раз и задал вопрос про оборудование, т.к. если бы падение
было зафиксировано на реальной системе вследствие багов в прошивке или
ещё каких нюансов, то к рассмотрению данной проблемы подошли бы уже с
другим прицелом.

Так что я бы рассмотрел возможность переквалифицирования данных сработок
в Won't fix, главное - с наличием соотв. обоснования в комментариях.
Одно дело, когда отсутствие проверки при работе с аппаратурой
действительно непонятно и вызывает лишь изумление, в таких случаях патчи
однозначно не будут лишними, но текущие сработки под такое вроде как не
подходят. Отсутствие проверок вполне понятно, верно?

Тем не менее, если сомнения в последовательности и понятности кода всё же
имеются или появятся в процессе дальнейшей кросс-верификации (для этих
предупреждений вроде не я делал), то подобные патчи сможем конечно
добавить в lvc-ветки. Но в master без явных доказательств необходимости
наличия проверок при работе с hardware они обычно это дело не трогают.