* [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
@ 2023-01-17 10:29 Dan Carpenter
2023-01-17 14:14 ` Cristian Marussi
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2023-01-17 10:29 UTC (permalink / raw)
To: cristian.marussi; +Cc: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
2023-01-17 10:29 [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack Dan Carpenter
@ 2023-01-17 14:14 ` Cristian Marussi
2023-01-18 15:41 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Cristian Marussi @ 2023-01-17 14:14 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-arm-kernel
On Tue, Jan 17, 2023 at 01:29:13PM +0300, Dan Carpenter wrote:
> Hello Cristian Marussi,
>
Hi Dan,
> 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/
>
Right. I'll fix dropping all unneded debugfs checks and repost as a
reply patch so that Sudeep can re-submit on next quickly.
I'm still puzzled why I regularly do not catch these errors on my smatch
setup running kchecker which does only reports me a bunch of errors on
core bitops code. (I have recently pulled updated smatch branch...)
Thanks,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
2023-01-17 14:14 ` Cristian Marussi
@ 2023-01-18 15:41 ` Dan Carpenter
2023-01-18 16:28 ` Cristian Marussi
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2023-01-18 15:41 UTC (permalink / raw)
To: Cristian Marussi; +Cc: linux-arm-kernel
On Tue, Jan 17, 2023 at 02:14:30PM +0000, Cristian Marussi wrote:
>
> I'm still puzzled why I regularly do not catch these errors on my smatch
> setup running kchecker which does only reports me a bunch of errors on
> core bitops code. (I have recently pulled updated smatch branch...)
This check requires the cross function database to work. Building the
cross function database is easy enough, but takes overnight.
~/smatch/smatch_scripts/build_kernel_data.sh
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
2023-01-18 15:41 ` Dan Carpenter
@ 2023-01-18 16:28 ` Cristian Marussi
2023-01-23 12:46 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Cristian Marussi @ 2023-01-18 16:28 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-arm-kernel
On Wed, Jan 18, 2023 at 06:41:19PM +0300, Dan Carpenter wrote:
> On Tue, Jan 17, 2023 at 02:14:30PM +0000, Cristian Marussi wrote:
> >
> > I'm still puzzled why I regularly do not catch these errors on my smatch
> > setup running kchecker which does only reports me a bunch of errors on
> > core bitops code. (I have recently pulled updated smatch branch...)
>
> This check requires the cross function database to work. Building the
> cross function database is easy enough, but takes overnight.
>
> ~/smatch/smatch_scripts/build_kernel_data.sh
>
Thanks I'll try.
BTW I'm using v1.73 and I also see since a while a lot of errors like:
internal error: file too large: db/kernel.return_fixes (limit 4096 bytes)
internal error: dev/src/pdsw/linux/arch/arm64/include/asm/jump_label.h:38 SQL error #2: UNIQUE constraint failed: mtag_data.tag, mtag_data.offset, mtag_data.type, mtag_data.value
internal error: dev/src/pdsw/linux/arch/arm64/include/asm/jump_label.h:38 SQL: 'insert into mtag_data values (7179031484371304448, 0, 8041, '1: b %l[l_yes] \n.pushsection __jump_table, "aw" \n .align 3 \n .long 1b - ., %l[l_yes] - . \n .quad %c0 - . \n.popsection \n ');'
Trying to revert back a few months of commits and those errors disappear.
Is it related to have stale DB ?
Thanks for you help,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack
2023-01-18 16:28 ` Cristian Marussi
@ 2023-01-23 12:46 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2023-01-23 12:46 UTC (permalink / raw)
To: Cristian Marussi; +Cc: linux-arm-kernel
On Wed, Jan 18, 2023 at 04:28:38PM +0000, Cristian Marussi wrote:
> On Wed, Jan 18, 2023 at 06:41:19PM +0300, Dan Carpenter wrote:
> > On Tue, Jan 17, 2023 at 02:14:30PM +0000, Cristian Marussi wrote:
> > >
> > > I'm still puzzled why I regularly do not catch these errors on my smatch
> > > setup running kchecker which does only reports me a bunch of errors on
> > > core bitops code. (I have recently pulled updated smatch branch...)
> >
> > This check requires the cross function database to work. Building the
> > cross function database is easy enough, but takes overnight.
> >
> > ~/smatch/smatch_scripts/build_kernel_data.sh
> >
>
> Thanks I'll try.
>
> BTW I'm using v1.73 and I also see since a while a lot of errors like:
>
> internal error: file too large: db/kernel.return_fixes (limit 4096 bytes)
>
That's an interesting thing. I had to make db/kernel.return_fixes
larger a few months back, but why is the db/kernel.return_fixes out of
sync with the code to parse it in v1.73?
> internal error: dev/src/pdsw/linux/arch/arm64/include/asm/jump_label.h:38 SQL error #2: UNIQUE constraint failed: mtag_data.tag, mtag_data.offset, mtag_data.type, mtag_data.value
>
> internal error: dev/src/pdsw/linux/arch/arm64/include/asm/jump_label.h:38 SQL: 'insert into mtag_data values (7179031484371304448, 0, 8041, '1: b %l[l_yes] \n.pushsection __jump_table, "aw" \n .align 3 \n .long 1b - ., %l[l_yes] - . \n .quad %c0 - . \n.popsection \n ');'
>
> Trying to revert back a few months of commits and those errors disappear.
> Is it related to have stale DB ?
Yep. Probably.
Can you retry on the latest git?
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-23 12:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-17 10:29 [bug report] firmware: arm_scmi: Call Raw mode hooks from the core stack Dan Carpenter
2023-01-17 14:14 ` Cristian Marussi
2023-01-18 15:41 ` Dan Carpenter
2023-01-18 16:28 ` Cristian Marussi
2023-01-23 12:46 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).