Re: [PATCH v2 3/3] arinc653: avoid array overrun

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.03.2026 21:43, Stewart Hildebrand wrote:
> On 3/25/26 09:22, Jürgen Groß wrote:
>> On 25.03.26 13:55, Jan Beulich wrote:
>>> Incrementing ->sched_index between bounds check and array access may
>>> result in accessing one past the array when that is fully filled
>>> (->num_schedule_entries == ARINC653_MAX_DOMAINS_PER_SCHEDULE).
>>>
>>> Fixes: 22787f2e107c ("ARINC 653 scheduler")
>>> Reported-by: Kamil Frankowicz <kamil.frankowicz@xxxxxxx>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
> 
> Reviewed-by: Stewart Hildebrand <stewart@xxxxxxx>
> 
> Thanks for this.
> 
>>
>> with ...
>>
>>> ---
>>> Jürgen, provided I understood him correctly, suggests that something like
>>>
>>>      while ( now >= sched_priv->next_switch_time )
>>>      {
>>>          sched_priv->sched_index++;
>>>          ASSERT(sched_priv->sched_index < sched_priv->num_schedule_entries);
>>>          sched_priv->next_switch_time +=
>>>              sched_priv->schedule[sched_priv->sched_index].runtime;
>>>      }
>>>
>>> should also be valid to move to, due to constraints applied by
>>> arinc653_sched_set().
> 
> Not quite, because major_frame is allowed to be larger than the sum of the
> runtimes, and in that case the ASSERT would trigger during the idle period.
> 
>>> I'm hesitant to make such a change though, not
>>> really knowing the scheduler; the change here looks more obviously correct
>>> to me. Albeit the Fixes: tag may thus want dropping.
>>
>> the Fixes: tag dropped, as the constraints mentioned are IMO really enough
>> to avoid an issue.
> 
> No, the constraints aren't enough, the out-of-bounds access would occur during
> an idle period of a fully filled schedule. I suggest keeping the Fixes: tag.

Jürgen, am I okay to keep your R-b with Fixes: re-added?

Jan