RE: [External Mail] Re: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread

[Wu. JackBB (GSM)]

Hi Simon,

After further internal review, we are correcting several
responses from our previous reply. The following items will
be fixed in v5.

On Jack Wu wrote:

>> [Severity: High]
>> Infinite busy loop on signal pending.

> This is intentional. Channel disable must complete or timeout.
> The loop does not busy-spin: each iteration sleeps in
> wait_event_interruptible_timeout until the condition is met or
> the timeout expires.

Will change wait_event_interruptible_timeout to
wait_event_timeout and remove the -ERESTARTSYS / goto
start_wait block.

>> [Severity: High]
>> Race in mtk_fsm_evt_submit — fsm_handler NULL after spinlock
>> release.

> The GATECLOSED check under evtq_lock ensures fsm_handler is
> valid. After the check passes, the event is submitted and the
> kthread is woken.

Will move wake_up_process inside evtq_lock in evt_submit, and
have mtk_fsm_exit clear fsm_handler under evtq_lock:

  evt_submit:
    wake_up_process(fsm->fsm_handler);
    spin_unlock_irqrestore(&fsm->evtq_lock, flags);

  mtk_fsm_exit:
    spin_lock_irqsave(&fsm->evtq_lock, flags);
    handler = fsm->fsm_handler;
    fsm->fsm_handler = NULL;
    spin_unlock_irqrestore(&fsm->evtq_lock, flags);
    if (handler)
        kthread_stop(handler);

>> [Severity: High]
>> rtft_entry->data passed to action callback before data_len
>> validation.

> The action functions only read fixed-size fields. The modem
> firmware is a trusted source.

Will move data_len validation before the callback call in
mtk_fsm_parse_hs2_msg, and pass data_len to the callback.

>> [Severity: High]
>> Uninitialized kernel heap memory in mtk_fsm_send_hs3_msg.

> The reserved[2] field is sent to the local modem over PCIe DMA,
> not exposed to external networks.

Will add memset(skb->data, 0, RTFT_DATA_SIZE) after
__dev_alloc_skb in mtk_fsm_send_hs3_msg.

>> [Severity: High]
>> Silent kcalloc failure in mtk_port_disable.

> Allocation failure is extremely unlikely. Hardware has already
> been power-cycled via PLDR.

Will replace radix_tree_gang_lookup with radix_tree_for_each_slot,
eliminating the temporary array. Same pattern as
mtk_port_search_by_name.

Thanks.

Jack Wu