From: Dan Carpenter <error27@gmail.com>
To: cristian.marussi@arm.com
Cc: linux-arm-kernel@lists.infradead.org
Subject: [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
Date: Tue, 17 Jan 2023 13:29:13 +0300 [thread overview]
Message-ID: <Y8Z4eaPAaCCkf0ut@kili> (raw)
Hello Cristian Marussi,
The patch ffb07d58dcba: "firmware: arm_scmi: Call Raw mode hooks from
the core stack" from Jan 13, 2023, leads to the following Smatch
static checker warning:
drivers/firmware/arm_scmi/driver.c:2732 scmi_probe()
error: 'info->dbg' dereferencing possible ERR_PTR()
drivers/firmware/arm_scmi/driver.c
2630 static int scmi_probe(struct platform_device *pdev)
2631 {
2632 int ret;
2633 struct scmi_handle *handle;
2634 const struct scmi_desc *desc;
2635 struct scmi_info *info;
2636 struct device *dev = &pdev->dev;
2637 struct device_node *child, *np = dev->of_node;
2638
2639 desc = of_device_get_match_data(dev);
2640 if (!desc)
2641 return -EINVAL;
2642
2643 info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
2644 if (!info)
2645 return -ENOMEM;
2646
2647 info->id = ida_alloc_min(&scmi_id, 0, GFP_KERNEL);
2648 if (info->id < 0)
2649 return info->id;
2650
2651 info->dev = dev;
2652 info->desc = desc;
2653 info->bus_nb.notifier_call = scmi_bus_notifier;
2654 info->dev_req_nb.notifier_call = scmi_device_request_notifier;
2655 INIT_LIST_HEAD(&info->node);
2656 idr_init(&info->protocols);
2657 mutex_init(&info->protocols_mtx);
2658 idr_init(&info->active_protocols);
2659 mutex_init(&info->devreq_mtx);
2660
2661 platform_set_drvdata(pdev, info);
2662 idr_init(&info->tx_idr);
2663 idr_init(&info->rx_idr);
2664
2665 handle = &info->handle;
2666 handle->dev = info->dev;
2667 handle->version = &info->version;
2668 handle->devm_protocol_acquire = scmi_devm_protocol_acquire;
2669 handle->devm_protocol_get = scmi_devm_protocol_get;
2670 handle->devm_protocol_put = scmi_devm_protocol_put;
2671
2672 /* System wide atomic threshold for atomic ops .. if any */
2673 if (!of_property_read_u32(np, "atomic-threshold-us",
2674 &info->atomic_threshold))
2675 dev_info(dev,
2676 "SCMI System wide atomic threshold set to %d us\n",
2677 info->atomic_threshold);
2678 handle->is_transport_atomic = scmi_is_transport_atomic;
2679
2680 if (desc->ops->link_supplier) {
2681 ret = desc->ops->link_supplier(dev);
2682 if (ret)
2683 goto clear_ida;
2684 }
2685
2686 /* Setup all channels described in the DT at first */
2687 ret = scmi_channels_setup(info);
2688 if (ret)
2689 goto clear_ida;
2690
2691 ret = bus_register_notifier(&scmi_bus_type, &info->bus_nb);
2692 if (ret)
2693 goto clear_txrx_setup;
2694
2695 ret = blocking_notifier_chain_register(&scmi_requested_devices_nh,
2696 &info->dev_req_nb);
2697 if (ret)
2698 goto clear_bus_notifier;
2699
2700 ret = scmi_xfer_info_init(info);
2701 if (ret)
2702 goto clear_dev_req_notifier;
2703
2704 if (scmi_top_dentry) {
2705 info->dbg = scmi_debugfs_common_setup(info);
The scmi_debugfs_common_setup() has messed up returns.
It returns both NULL and error pointers for errors. It checks debugfs
functions and the returns for those are generally supposed to be
ignored. I have written a blog about mixing error pointers and NULL
which also explains this historical/psychology based reason why debugfs
does not follow the normal pattern:
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
2706
2707 if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) && info->dbg) {
2708 int id, num_chans = 0;
2709 struct scmi_chan_info *cinfo;
2710 u8 channels[SCMI_MAX_CHANNELS] = {};
2711 DECLARE_BITMAP(protos, SCMI_MAX_CHANNELS) = {};
2712
2713 /* Enumerate all channels to collect their ids */
2714 idr_for_each_entry(&info->tx_idr, cinfo, id) {
2715 /*
2716 * Cannot happen, but be defensive.
2717 * Zero as num_chans is ok, warn and carry on.
2718 */
2719 if (num_chans >= SCMI_MAX_CHANNELS || !cinfo) {
2720 dev_warn(dev,
2721 "SCMI RAW - Error enumerating channels\n");
2722 break;
2723 }
2724
2725 if (!test_bit(cinfo->id, protos)) {
2726 channels[num_chans++] = cinfo->id;
2727 set_bit(cinfo->id, protos);
2728 }
2729 }
2730
2731 info->raw = scmi_raw_mode_init(handle,
--> 2732 info->dbg->top_dentry,
^^^^^^^^^^^
Error pointer dereference.
2733 info->id,
2734 channels, num_chans,
2735 info->desc,
2736 info->tx_minfo.max_msg);
2737 if (IS_ERR(info->raw)) {
2738 dev_err(dev, "Failed to initialize SCMI RAW Mode !\n");
2739
2740 ret = PTR_ERR(info->raw);
2741 }
2742
2743 if (!IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX)) {
2744 if (!ret)
2745 return ret;
2746 /*
2747 * Bail out if we failed to init raw mode and
2748 * RAW_MODE_SUPPORT_COEX was not configured.
2749 */
2750 goto clear_dev_req_notifier;
2751 }
2752
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2023-01-17 10:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-17 10:29 Dan Carpenter [this message]
2023-01-17 14:14 ` [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack Cristian Marussi
2023-01-18 15:41 ` Dan Carpenter
2023-01-18 16:28 ` Cristian Marussi
2023-01-23 12:46 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y8Z4eaPAaCCkf0ut@kili \
--to=error27@gmail.com \
--cc=cristian.marussi@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.