[lvc-project] [PATCH net v2 3/3] net: fealnx: allocate the card index from an IDA

[Fedor Pchelkin]

On Fri, 02. Oct 22:28, Andrew Lunn wrote:
> On Fri, Oct 02, 2026 at 02:10:10PM +0000, Жамбакиев Радий Рикардинович wrote:
> > From: Radiy Zhambakiev <r.zhambakiev at prosoftsystems.ru>
> > 
> > card_idx is a static counter that is incremented on every probe.
> > It can overflow and wrap to a negative value, which then indexes
> > options[] and full_duplex[] out of bounds. Large values also no
> > longer fit in the 12-byte boardname[] buffer.
> > 
> > Allocate the card index from an IDA and free it on probe failure and
> > remove. The IDA reuses ids on re-add, preserving the options[] and
> > full_duplex[] mapping by probe order.
> > 
> > Store the id in the driver-private data so fealnx_remove_one() can
> > free it, and size boardname to hold a full 32-bit id.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> How have you tested this?
> 
> The advantage of the KISS approach is it is stupid, so unlikely to be
> wrong. The complexity here is much higher, so it is more likely to be
> wrong. That is something we have to considered.
> 
> Have you put it into a loop, and probed it 10342432341 times, on real
> hardware?

Ясно, тут будет трудно так продвинуть идею с IDA.  Сделайте, пожалуйста,
как он предлагает, и посмотрим, примут ли такое.  Спорить из-за этого
древнего драйвера для древних устройств (который однако включен в
конфигах сертифицируемых продуктов многих участников), не стоит
потраченного времени.

В формулировках патчей ещё посмотрите, чтобы не было того, к чему иишка
придерётся в контексте teardown rx-tx engines.

Для патча, на который был дан Reviewed-by и который не меняется для
след. версии, добавьте этот тег Reviewed-by над своей подписью
Signed-off-by.

Кстати насчёт

  - [Medium] The commit message says the IDA "reuses ids on re-add,
  preserving the options[] and full_duplex[] mapping by probe order".

В описании патча стоило это явно прописать, но мне кажется это не баг, а
фича.  И говорит за ещё один недостаток подхода с обычным инкрементом
счётчика.  Если у нас есть одна физическая карта, и мы делаем
probe-remove несколько раз, то у этой карты почему-то увеличивается
card_idx, хотя это всё та же физическая карта, т.е. тот же физический
юнит, если говорить относительно того, зачем этот индекс вообще нужен -
для итерации по

/* Used to pass the media type, etc.                            */
/* Both 'options[]' and 'full_duplex[]' should exist for driver */
/* interoperability.                                            */
/* The media type is usually passed in 'options[]'.             */
#define MAX_UNITS 8		/* More are supported, limit only on options */
static int options[MAX_UNITS] = { -1, -1, -1, -1, -1, -1, -1, -1 };
static int full_duplex[MAX_UNITS] = { -1, -1, -1, -1, -1, -1, -1, -1 };