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
next prev parent 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.