Skip to contentSkip to conversation

net: wwan: t7xx: Add control port

bug report Dan Carpenter 2 participants 2 messages May 17, 2022 – May 19, 2022 1 message not in the archive Arm MediaTek

lore
Share & tools
lore
#0 lore raw

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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek

A message that is not in the archive (sent off-list or not yet received): KL1PR03MB58335FE9D2D6595FABD14996E4D19@KL1PR03MB5833.apcprd03.prod.outlook.com

#2 lore raw

Hi,

Fixes submitted to net-next:

https://lore.kernel.org/netdev/[email protected]/

Thanks,

Ricardo

Quoted 101 lines · Haijun Liu (刘海军)

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 <[email protected]>
Sent: Tuesday, May 17, 2022 7:50 PM
To: Haijun Liu (刘海军) <[email protected]>
Cc: [email protected]
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

Quoted 12 lines

************* 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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek