[bug report] net: wwan: t7xx: Add control port

[bug report] net: wwan: t7xx: Add control port

[Dan Carpenter]

Hello Haijun Liu,

The patch da45d2566a1d: "net: wwan: t7xx: Add control port" from May
6, 2022, leads to the following Smatch static checker warning:

	drivers/net/wwan/t7xx/t7xx_modem_ops.c:499 t7xx_core_hk_handler()
	error: potentially dereferencing uninitialized 'event'.

drivers/net/wwan/t7xx/t7xx_modem_ops.c
    457 static void t7xx_core_hk_handler(struct t7xx_modem *md, struct t7xx_fsm_ctl *ctl,
    458                                  enum t7xx_fsm_event_state event_id,
    459                                  enum t7xx_fsm_event_state err_detect)
    460 {
    461         struct t7xx_sys_info *core_info = &md->core_md;
    462         struct device *dev = &md->t7xx_dev->pdev->dev;
    463         struct t7xx_fsm_event *event, *event_next;
                                       ^^^^^


    464         unsigned long flags;
    465         int ret;
    466 
    467         t7xx_prepare_host_rt_data_query(core_info);
    468 
    469         while (!kthread_should_stop()) {
                        ^^^^^^^^^^^^^^^^^^^^^
What about this is true on the first iteration through the loop?

    470                 bool event_received = false;
    471 
    472                 spin_lock_irqsave(&ctl->event_lock, flags);
    473                 list_for_each_entry_safe(event, event_next, &ctl->event_queue, entry) {
    474                         if (event->event_id == err_detect) {
    475                                 list_del(&event->entry);
    476                                 spin_unlock_irqrestore(&ctl->event_lock, flags);
    477                                 dev_err(dev, "Core handshake error event received\n");
    478                                 goto err_free_event;
    479                         } else if (event->event_id == event_id) {
    480                                 list_del(&event->entry);
    481                                 event_received = true;
    482                                 break;
    483                         }
    484                 }
    485                 spin_unlock_irqrestore(&ctl->event_lock, flags);
    486 
    487                 if (event_received)
    488                         break;
    489 
    490                 wait_event_interruptible(ctl->event_wq, !list_empty(&ctl->event_queue) ||
    491                                          kthread_should_stop());
    492                 if (kthread_should_stop())
    493                         goto err_free_event;
    494         }
    495 
    496         if (ctl->exp_flg)
    497                 goto err_free_event;
                        ^^^^^^^^^^^^^^^^^^^
Uninitialized on this path too.

    498 
--> 499         ret = t7xx_parse_host_rt_data(ctl, core_info, dev, event->data, event->length);
                                                                   ^^^^^^^
Uninitialized?

    500         if (ret) {
    501                 dev_err(dev, "Host failure parsing runtime data: %d\n", ret);
    502                 goto err_free_event;
    503         }
    504 
    505         if (ctl->exp_flg)
    506                 goto err_free_event;
    507 
    508         ret = t7xx_prepare_device_rt_data(core_info, dev, event->data);
    509         if (ret) {
    510                 dev_err(dev, "Device failure parsing runtime data: %d", ret);
    511                 goto err_free_event;
    512         }
    513 
    514         core_info->ready = true;
    515         core_info->handshake_ongoing = false;
    516         wake_up(&ctl->async_hk_wq);
    517 err_free_event:
    518         kfree(event);
    519 }

regards,
dan carpenter

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [bug report] net: wwan: t7xx: Add control port

[Martinez, Ricardo]

Hi,

Fixes submitted to net-next:

https://lore.kernel.org/netdev/20220518195529.126246-1-ricardo.martinez@linux.intel.com/


Thanks,

Ricardo

On 5/17/2022 6:23 PM, Haijun Liu (刘海军) wrote:
> Dear Dan,
> Thanks!
>
> Dear Ricardo,
> May we help to check the Smatch checker warning?
> Seems also happened on DMA/Data path HW patch~
>
> BR
> Haijun
>
> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Tuesday, May 17, 2022 7:50 PM
> To: Haijun Liu (刘海军) <haijun.liu@mediatek.com>
> Cc: linux-mediatek@lists.infradead.org
> Subject: [bug report] net: wwan: t7xx: Add control port
>
> Hello Haijun Liu,
>
> The patch da45d2566a1d: "net: wwan: t7xx: Add control port" from May 6, 2022, leads to the following Smatch static checker warning:
>
> drivers/net/wwan/t7xx/t7xx_modem_ops.c:499 t7xx_core_hk_handler()
> error: potentially dereferencing uninitialized 'event'.
>
> drivers/net/wwan/t7xx/t7xx_modem_ops.c
>      457 static void t7xx_core_hk_handler(struct t7xx_modem *md, struct t7xx_fsm_ctl *ctl,
>      458                                  enum t7xx_fsm_event_state event_id,
>      459                                  enum t7xx_fsm_event_state err_detect)
>      460 {
>      461         struct t7xx_sys_info *core_info = &md->core_md;
>      462         struct device *dev = &md->t7xx_dev->pdev->dev;
>      463         struct t7xx_fsm_event *event, *event_next;
>                                         ^^^^^
>
>
>      464         unsigned long flags;
>      465         int ret;
>      466
>      467         t7xx_prepare_host_rt_data_query(core_info);
>      468
>      469         while (!kthread_should_stop()) {
>                          ^^^^^^^^^^^^^^^^^^^^^ What about this is true on the first iteration through the loop?
>
>      470                 bool event_received = false;
>      471
>      472                 spin_lock_irqsave(&ctl->event_lock, flags);
>      473                 list_for_each_entry_safe(event, event_next, &ctl->event_queue, entry) {
>      474                         if (event->event_id == err_detect) {
>      475                                 list_del(&event->entry);
>      476                                 spin_unlock_irqrestore(&ctl->event_lock, flags);
>      477                                 dev_err(dev, "Core handshake error event received\n");
>      478                                 goto err_free_event;
>      479                         } else if (event->event_id == event_id) {
>      480                                 list_del(&event->entry);
>      481                                 event_received = true;
>      482                                 break;
>      483                         }
>      484                 }
>      485                 spin_unlock_irqrestore(&ctl->event_lock, flags);
>      486
>      487                 if (event_received)
>      488                         break;
>      489
>      490                 wait_event_interruptible(ctl->event_wq, !list_empty(&ctl->event_queue) ||
>      491                                          kthread_should_stop());
>      492                 if (kthread_should_stop())
>      493                         goto err_free_event;
>      494         }
>      495
>      496         if (ctl->exp_flg)
>      497                 goto err_free_event;
>                          ^^^^^^^^^^^^^^^^^^^ Uninitialized on this path too.
>
>      498
> --> 499         ret = t7xx_parse_host_rt_data(ctl, core_info, dev, event->data, event->length);
>                                                                     ^^^^^^^ Uninitialized?
>
>      500         if (ret) {
>      501                 dev_err(dev, "Host failure parsing runtime data: %d\n", ret);
>      502                 goto err_free_event;
>      503         }
>      504
>      505         if (ctl->exp_flg)
>      506                 goto err_free_event;
>      507
>      508         ret = t7xx_prepare_device_rt_data(core_info, dev, event->data);
>      509         if (ret) {
>      510                 dev_err(dev, "Device failure parsing runtime data: %d", ret);
>      511                 goto err_free_event;
>      512         }
>      513
>      514         core_info->ready = true;
>      515         core_info->handshake_ongoing = false;
>      516         wake_up(&ctl->async_hk_wq);
>      517 err_free_event:
>      518         kfree(event);
>      519 }
>
> regards,
> dan carpenter
>
> ************* MEDIATEK Confidentiality Notice ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek