All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Masney <bmasney@redhat.com>
To: Conor Dooley <conor@kernel.org>
Cc: linux-clk@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	Daire McNamara <daire.mcnamara@microchip.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] clk: microchip: mpfs-ccc: fix peripheral driver registration failures after oob fix
Date: Thu, 30 Apr 2026 18:42:06 -0400	[thread overview]
Message-ID: <afPavvW89sHTx644@redhat.com> (raw)
In-Reply-To: <20260430-unmade-overpay-28d175fd09a3@spud>

On Thu, Apr 30, 2026 at 07:30:28PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Commit 2f7ae8ab6aa73 ("clk: microchip: mpfs-ccc: fix out of bounds
> access during output registration") fixed the out of bounds access, but
> it did so by packing sparse indices into a linear space. When
> peripheral drivers request clocks, they obviously don't care for this
> compression and use the sparse indices, and therefore try to request the
> wrong clocks or clocks that don't exist.
> 
> The most straightforward fix here seems to stop being clever with the
> packing and just overallocate the array.
> 
> Fixes: 2f7ae8ab6aa73 ("clk: microchip: mpfs-ccc: fix out of bounds access during output registration")
> Fixes: d39fb172760e ("clk: microchip: add PolarFire SoC fabric clock support")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> CC: Daire McNamara <daire.mcnamara@microchip.com>
> CC: Michael Turquette <mturquette@baylibre.com>
> CC: Stephen Boyd <sboyd@kernel.org>
> CC: Brian Masney <bmasney@redhat.com>
> CC: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> CC: linux-riscv@lists.infradead.org
> CC: linux-clk@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/clk/microchip/clk-mpfs-ccc.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/microchip/clk-mpfs-ccc.c b/drivers/clk/microchip/clk-mpfs-ccc.c
> index 0a76a1aaa50f7..40c17593e5941 100644
> --- a/drivers/clk/microchip/clk-mpfs-ccc.c
> +++ b/drivers/clk/microchip/clk-mpfs-ccc.c
> @@ -32,6 +32,7 @@
>  #define MPFS_CCC_FIXED_DIV		4
>  #define MPFS_CCC_OUTPUTS_PER_PLL	4
>  #define MPFS_CCC_REFS_PER_PLL		2
> +#define MPFS_CCC_NUM_CLKS		16
>  
>  struct mpfs_ccc_data {
>  	void __iomem **pll_base;
> @@ -178,7 +179,7 @@ static int mpfs_ccc_register_outputs(struct device *dev, struct mpfs_ccc_out_hw_
>  			return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
>  					     out_hw->id);
>  
> -		data->hw_data.hws[out_hw->id - 2] = &out_hw->divider.hw;
> +		data->hw_data.hws[out_hw->id] = &out_hw->divider.hw;
>  	}
>  
>  	return 0;
> @@ -231,17 +232,9 @@ static int mpfs_ccc_probe(struct platform_device *pdev)
>  {
>  	struct mpfs_ccc_data *clk_data;
>  	void __iomem *pll_base[ARRAY_SIZE(mpfs_ccc_pll_clks)];
> -	unsigned int num_clks;
>  	int ret;
>  
> -	/*
> -	 * If DLLs get added here, mpfs_ccc_register_outputs() currently packs
> -	 * sparse clock IDs in the hws array
> -	 */
> -	num_clks = ARRAY_SIZE(mpfs_ccc_pll_clks) + ARRAY_SIZE(mpfs_ccc_pll0out_clks) +
> -		   ARRAY_SIZE(mpfs_ccc_pll1out_clks);
> -
> -	clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hw_data.hws, num_clks),
> +	clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hw_data.hws, MPFS_CCC_NUM_CLKS),
>  				GFP_KERNEL);
>  	if (!clk_data)
>  		return -ENOMEM;
> @@ -255,7 +248,7 @@ static int mpfs_ccc_probe(struct platform_device *pdev)
>  		return PTR_ERR(pll_base[1]);
>  
>  	clk_data->pll_base = pll_base;
> -	clk_data->hw_data.num = num_clks;
> +	clk_data->hw_data.num = MPFS_CCC_NUM_CLKS;
>  	clk_data->dev = &pdev->dev;
>  
>  	ret = mpfs_ccc_register_plls(clk_data->dev, mpfs_ccc_pll_clks,

Reviewed-by: Brian Masney <bmasney@redhat.com>

I confirmed that there are 16 clock IDs defined at the bottom of
include/dt-bindings/clock/microchip,mpfs-clock.h.

Brian


WARNING: multiple messages have this Message-ID (diff)
From: Brian Masney <bmasney@redhat.com>
To: Conor Dooley <conor@kernel.org>
Cc: linux-clk@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	Daire McNamara <daire.mcnamara@microchip.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] clk: microchip: mpfs-ccc: fix peripheral driver registration failures after oob fix
Date: Thu, 30 Apr 2026 18:42:06 -0400	[thread overview]
Message-ID: <afPavvW89sHTx644@redhat.com> (raw)
In-Reply-To: <20260430-unmade-overpay-28d175fd09a3@spud>

On Thu, Apr 30, 2026 at 07:30:28PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Commit 2f7ae8ab6aa73 ("clk: microchip: mpfs-ccc: fix out of bounds
> access during output registration") fixed the out of bounds access, but
> it did so by packing sparse indices into a linear space. When
> peripheral drivers request clocks, they obviously don't care for this
> compression and use the sparse indices, and therefore try to request the
> wrong clocks or clocks that don't exist.
> 
> The most straightforward fix here seems to stop being clever with the
> packing and just overallocate the array.
> 
> Fixes: 2f7ae8ab6aa73 ("clk: microchip: mpfs-ccc: fix out of bounds access during output registration")
> Fixes: d39fb172760e ("clk: microchip: add PolarFire SoC fabric clock support")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> CC: Daire McNamara <daire.mcnamara@microchip.com>
> CC: Michael Turquette <mturquette@baylibre.com>
> CC: Stephen Boyd <sboyd@kernel.org>
> CC: Brian Masney <bmasney@redhat.com>
> CC: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> CC: linux-riscv@lists.infradead.org
> CC: linux-clk@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/clk/microchip/clk-mpfs-ccc.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/microchip/clk-mpfs-ccc.c b/drivers/clk/microchip/clk-mpfs-ccc.c
> index 0a76a1aaa50f7..40c17593e5941 100644
> --- a/drivers/clk/microchip/clk-mpfs-ccc.c
> +++ b/drivers/clk/microchip/clk-mpfs-ccc.c
> @@ -32,6 +32,7 @@
>  #define MPFS_CCC_FIXED_DIV		4
>  #define MPFS_CCC_OUTPUTS_PER_PLL	4
>  #define MPFS_CCC_REFS_PER_PLL		2
> +#define MPFS_CCC_NUM_CLKS		16
>  
>  struct mpfs_ccc_data {
>  	void __iomem **pll_base;
> @@ -178,7 +179,7 @@ static int mpfs_ccc_register_outputs(struct device *dev, struct mpfs_ccc_out_hw_
>  			return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
>  					     out_hw->id);
>  
> -		data->hw_data.hws[out_hw->id - 2] = &out_hw->divider.hw;
> +		data->hw_data.hws[out_hw->id] = &out_hw->divider.hw;
>  	}
>  
>  	return 0;
> @@ -231,17 +232,9 @@ static int mpfs_ccc_probe(struct platform_device *pdev)
>  {
>  	struct mpfs_ccc_data *clk_data;
>  	void __iomem *pll_base[ARRAY_SIZE(mpfs_ccc_pll_clks)];
> -	unsigned int num_clks;
>  	int ret;
>  
> -	/*
> -	 * If DLLs get added here, mpfs_ccc_register_outputs() currently packs
> -	 * sparse clock IDs in the hws array
> -	 */
> -	num_clks = ARRAY_SIZE(mpfs_ccc_pll_clks) + ARRAY_SIZE(mpfs_ccc_pll0out_clks) +
> -		   ARRAY_SIZE(mpfs_ccc_pll1out_clks);
> -
> -	clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hw_data.hws, num_clks),
> +	clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hw_data.hws, MPFS_CCC_NUM_CLKS),
>  				GFP_KERNEL);
>  	if (!clk_data)
>  		return -ENOMEM;
> @@ -255,7 +248,7 @@ static int mpfs_ccc_probe(struct platform_device *pdev)
>  		return PTR_ERR(pll_base[1]);
>  
>  	clk_data->pll_base = pll_base;
> -	clk_data->hw_data.num = num_clks;
> +	clk_data->hw_data.num = MPFS_CCC_NUM_CLKS;
>  	clk_data->dev = &pdev->dev;
>  
>  	ret = mpfs_ccc_register_plls(clk_data->dev, mpfs_ccc_pll_clks,

Reviewed-by: Brian Masney <bmasney@redhat.com>

I confirmed that there are 16 clock IDs defined at the bottom of
include/dt-bindings/clock/microchip,mpfs-clock.h.

Brian


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

  reply	other threads:[~2026-04-30 22:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30 18:30 [PATCH v1] clk: microchip: mpfs-ccc: fix peripheral driver registration failures after oob fix Conor Dooley
2026-04-30 18:30 ` Conor Dooley
2026-04-30 22:42 ` Brian Masney [this message]
2026-04-30 22:42   ` Brian Masney

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=afPavvW89sHTx644@redhat.com \
    --to=bmasney@redhat.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=daire.mcnamara@microchip.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.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.