[PATCH net-next 0/3] net: Make sure clk_init_data is fully initialized

[PATCH net-next 0/3] net: Make sure clk_init_data is fully initialized

[Geert Uytterhoeven]

	Hi all,

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need.  However,
not initializing all members may cause subtle issues, which are only
exposed when CONFIG_INIT_STACK_ALL_PATTERN or CONFIG_INIT_STACK_NONE is
enabled.

Hence this series aims to make sure all members are fully initialized,
to avoid such bugs, and to prevent future breakage when converting
drivers to a different method for specifying the parents.

Part One[1] fixed all cases that I identified to be real bugs, in
response to a crash I saw on BeagleBone Black.

This series is the networking subpart of Part Two, which fixes remaining
cases that are currently harmless.  These are still fragile, and may
cause future breakage when converting drivers to a different method for
specifying the parents.

Thanks for your comments!

[1] "[PATCH treewide 0/5] clk: Make sure clk_init_data is fully
     initialized (part 1)"
    https://lore.kernel.org/cover.1787165329.git.geert+renesas@glider.be

Geert Uytterhoeven (3):
  net: macb: Make sure clk_init_data is fully initialized
  net: mdio: mux-meson-g12a: Make sure clk_init_data is fully
    initialized
  net: phy: air_en8811h: Make sure clk_init_data is fully initialized

 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 drivers/net/mdio/mdio-mux-meson-g12a.c   | 2 +-
 drivers/net/phy/air_en8811h.c            | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.43.0

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

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

[PATCH net-next 1/3] net: macb: Make sure clk_init_data is fully initialized

[Geert Uytterhoeven]

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need.  However,
not initializing all members may cause subtle issues, which are only
exposed when CONFIG_INIT_STACK_ALL_PATTERN or CONFIG_INIT_STACK_NONE is
enabled.

Make sure all members are fully initialized, to avoid such bugs, and to
prevent future breakage when converting drivers to a different method
for specifying the parents.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 1476bce77f34d2a5..319e43098f57ed7b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5376,7 +5376,7 @@ static int fu540_c000_clk_init(struct platform_device *pdev, struct clk **pclk,
 			       struct clk **hclk, struct clk **tx_clk,
 			       struct clk **rx_clk, struct clk **tsu_clk)
 {
-	struct clk_init_data init;
+	struct clk_init_data init = {};
 	int err = 0;
 
 	err = macb_clk_init_dflt(pdev, pclk, hclk, tx_clk, rx_clk, tsu_clk);
-- 
2.43.0


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

[PATCH net-next 2/3] net: mdio: mux-meson-g12a: Make sure clk_init_data is fully initialized

[Geert Uytterhoeven]

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need.  However,
not initializing all members may cause subtle issues, which are only
exposed when CONFIG_INIT_STACK_ALL_PATTERN or CONFIG_INIT_STACK_NONE is
enabled.

Make sure all members are fully initialized, to avoid such bugs, and to
prevent future breakage when converting drivers to a different method
for specifying the parents.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
---
 drivers/net/mdio/mdio-mux-meson-g12a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mdio/mdio-mux-meson-g12a.c b/drivers/net/mdio/mdio-mux-meson-g12a.c
index 08d6a6c93fb8bf02..e6c9ef3641afa27e 100644
--- a/drivers/net/mdio/mdio-mux-meson-g12a.c
+++ b/drivers/net/mdio/mdio-mux-meson-g12a.c
@@ -223,7 +223,7 @@ static int g12a_ephy_glue_clk_register(struct device *dev)
 {
 	struct g12a_mdio_mux *priv = dev_get_drvdata(dev);
 	const char *parent_names[PLL_MUX_NUM_PARENT];
-	struct clk_init_data init;
+	struct clk_init_data init = {};
 	struct g12a_ephy_pll *pll;
 	struct clk_mux *mux;
 	struct clk *clk;
-- 
2.43.0


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

[PATCH net-next 3/3] net: phy: air_en8811h: Make sure clk_init_data is fully initialized

[Geert Uytterhoeven]

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need.  However,
not initializing all members may cause subtle issues, which are only
exposed when CONFIG_INIT_STACK_ALL_PATTERN or CONFIG_INIT_STACK_NONE is
enabled.

Make sure all members are fully initialized, to avoid such bugs, and to
prevent future breakage when converting drivers to a different method
for specifying the parents.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
---
 drivers/net/phy/air_en8811h.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index edd49c193e476838..d1ab8744288bba0c 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -932,7 +932,7 @@ static const struct clk_ops an8811hb_clk_ops = {
 
 static int an8811hb_clk_provider_setup(struct device *dev, struct clk_hw *hw)
 {
-	struct clk_init_data init;
+	struct clk_init_data init = {};
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_COMMON_CLK))
@@ -1031,7 +1031,7 @@ static const struct clk_ops en8811h_clk_ops = {
 
 static int en8811h_clk_provider_setup(struct device *dev, struct clk_hw *hw)
 {
-	struct clk_init_data init;
+	struct clk_init_data init = {};
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_COMMON_CLK))
-- 
2.43.0


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

Re: [PATCH net-next 1/3] net: macb: Make sure clk_init_data is fully initialized

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The TX error recovery task modifies TX ring pointers and state without holding `queue->tx_ptr_lock`, racing with `macb_start_xmit`.
--

--- Patch [1]: [PATCH net-next 1/3] net: macb: Make sure clk_init_data is fully initialized ---
commit 72f05b5fbec22828f860396d626f5a6bbaacd336
Author: Geert Uytterhoeven <geert+renesas@glider.be>

net: macb: Make sure clk_init_data is fully initialized

This commit initializes the clk_init_data structure to prevent issues with
uninitialized mutually-exclusive members when stack initialization patterns
are enabled.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 1476bce77f34d..319e43098f57e 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing data
race between macb_tx_error_task() and macb_start_xmit() in this file.

Does macb_tx_error_task() race with macb_start_xmit() by modifying TX ring
pointers without holding queue->tx_ptr_lock?

In macb_tx_error_task(), the ring pointers are reset without a lock:

    queue->tx_head = 0;
    queue->tx_tail = 0;

Concurrently, macb_start_xmit() acquires the lock and updates tx_head:

    spin_lock_irqsave(&queue->tx_ptr_lock, flags);

Since netif_tx_stop_all_queues() only marks the queue as stopped but does not
wait for an already-executing ndo_start_xmit() to finish, could this race
corrupt the software TX ring pointers? 

This might lead to out-of-bounds DMA or use-after-free when the
desynchronized ring is subsequently processed after a transmission error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787240118.git.geert+renesas@glider.be?part=1

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

Re: [PATCH net-next 2/3] net: mdio: mux-meson-g12a: Make sure clk_init_data is fully initialized

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Busy-wait loop of up to 1 second with interrupts disabled in atomic clock enable callback
--

--- Patch [2]: [PATCH net-next 2/3] net: mdio: mux-meson-g12a: Make sure clk_init_data is fully initialized ---
commit 061a72e517f35ad4bc41a274d2b768db77d84ad1
Author: Geert Uytterhoeven <geert+renesas@glider.be>
Subject: net: mdio: mux-meson-g12a: Make sure clk_init_data is fully initialized

This commit ensures the clk_init_data structure is fully initialized by
adding an empty initializer. This prevents subtle bugs related to mutually
exclusive members when different stack initialization patterns are enabled.

> diff --git a/drivers/net/mdio/mdio-mux-meson-g12a.c b/drivers/net/mdio/mdio-mux-meson-g12a.c
> --- a/drivers/net/mdio/mdio-mux-meson-g12a.c
> +++ b/drivers/net/mdio/mdio-mux-meson-g12a.c
> @@ -223,7 +223,7 @@ static int g12a_ephy_glue_clk_register(struct device *dev)
>  {
>  	struct g12a_mdio_mux *priv = dev_get_drvdata(dev);
>  	const char *parent_names[PLL_MUX_NUM_PARENT];
> -	struct clk_init_data init;
> +	struct clk_init_data init = {};

[Severity: High]
This is a pre-existing issue, but while reviewing this clock initialization, 
I noticed a potential lockup in the related PLL operations.

Does g12a_ephy_pll_enable() risk triggering the hard lockup detector?

The g12a_ephy_pll_ops struct is used later in this function to initialize 
the pll clock, and its .enable callback maps to g12a_ephy_pll_enable():

drivers/net/mdio/mdio-mux-meson-g12a.c:g12a_ephy_pll_enable() {
    ...
    return readl_poll_timeout(pll->base + ETH_PLL_CTL0, val,
                              val & PLL_CTL0_LOCK_DIG, 0, PLL_LOCK_TIMEOUT);
}

The clock framework calls the .enable callback with the enable_lock spinlock
held and local interrupts disabled. Since the delay parameter is 0 and 
PLL_LOCK_TIMEOUT is 1,000,000, readl_poll_timeout() evaluates to a tight
busy-wait loop. 

If the PLL hardware fails to lock or takes a long time, could this stall 
the CPU for up to 1 second in an interrupt-disabled context?

Would it be possible to move this hardware initialization sequence to the 
.prepare callback instead, which executes in a preemptible context where 
sleeping delays are permitted?

>  	struct g12a_ephy_pll *pll;
>  	struct clk_mux *mux;
>  	struct clk *clk;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787240118.git.geert+renesas@glider.be?part=2

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

Re: [PATCH net-next 3/3] net: phy: air_en8811h: Make sure clk_init_data is fully initialized

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The PHY driver calls the global `clk_save_context()` and `clk_restore_context()` APIs in its suspend and resume paths, forcefully saving and restoring all clocks in the entire system.
--

--- Patch [3]: [PATCH net-next 3/3] net: phy: air_en8811h: Make sure clk_init_data is fully initialized ---
commit 681a33786ef724d10dc7ce4f726e8ec894967ea4
Author: Geert Uytterhoeven <geert+renesas@glider.be>

net: phy: air_en8811h: Make sure clk_init_data is fully initialized

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need. However,
not initializing all members may cause subtle issues. This commit makes
sure all members are fully initialized.

> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index edd49c193e476..d1ab8744288bb 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c

[ ... ]

> @@ -1031,7 +1031,7 @@ static const struct clk_ops en8811h_clk_ops = {
>  
>  static int en8811h_clk_provider_setup(struct device *dev, struct clk_hw *hw)
>  {
> -	struct clk_init_data init;
> +	struct clk_init_data init = {};
>  	int ret;
>  
>  	if (!IS_ENABLED(CONFIG_COMMON_CLK))

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this driver's 
clock handling I noticed a potential power management issue further down in 
the file.

In drivers/net/phy/air_en8811h.c:

static int en8811h_resume(struct phy_device *phydev)
{
	clk_restore_context();

	return genphy_resume(phydev);
}

static int en8811h_suspend(struct phy_device *phydev)
{
	clk_save_context();

	return genphy_suspend(phydev);
}

Is it safe for the PHY driver to call the global clk_save_context() and 
clk_restore_context() APIs during its suspend and resume handlers?

These APIs are intended for core PM operations and iterate over the entire 
system's clock tree to save or restore state for all clocks system-wide.

Could restoring all clocks globally whenever this single leaf PHY resumes
forcefully reset or disrupt the clocks of completely unrelated devices,
potentially causing system lockups or crashes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787240118.git.geert+renesas@glider.be?part=3

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

Re: [PATCH net-next 0/3] net: Make sure clk_init_data is fully initialized

[Jakub Kicinski]

On Fri, 21 Aug 2026 12:07:29 +0200 Geert Uytterhoeven wrote:
>  drivers/net/ethernet/cadence/macb_main.c | 2 +-
>  drivers/net/mdio/mdio-mux-meson-g12a.c   | 2 +-
>  drivers/net/phy/air_en8811h.c            | 4 ++--

Hi Geert!

Unfortunately we don't queue -next patches during the merge window,
you'll have to repsot.
-- 
pw-bot: defer

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