This series tries to fix and improvement the Raspberry Pi firmware clock
driver. This mostly focus on clock discovery mechanism.
Just a note patch #3 depends on patch #2.
Stefan Wahren (3):
clk: bcm: rpi: Prevent out-of-bounds access
clk: bcm: rpi: Add missing newline
clk: bcm: rpi: Show clock id limit in error case
drivers/clk/bcm/clk-raspberrypi.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
--
2.25.1
The while loop in raspberrypi_discover_clocks() relies on the assumption
that the id of the last clock element is zero. Because this data comes
from the Videocore firmware and it doesn't guarantuee such a behavior
this could lead to out-of-bounds access. So fix this by providing
a sentinel element.
Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
Link: https://github.com/raspberrypi/firmware/issues/1688
Suggested-by: Phil Elwell <[email protected]>
Signed-off-by: Stefan Wahren <[email protected]>
---
drivers/clk/bcm/clk-raspberrypi.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 73518009a0f2..79cbf0c0b401 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -344,8 +344,13 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
struct rpi_firmware_get_clocks_response *clks;
int ret;
+ /*
+ * The firmware doesn't guarantee that the last element of
+ * RPI_FIRMWARE_GET_CLOCKS is zeroed. So allocate an additional
+ * zero element as sentinel.
+ */
clks = devm_kcalloc(rpi->dev,
- RPI_FIRMWARE_NUM_CLK_ID, sizeof(*clks),
+ RPI_FIRMWARE_NUM_CLK_ID + 1, sizeof(*clks),
GFP_KERNEL);
if (!clks)
return -ENOMEM;
--
2.25.1
Some log messages lacks the final newline. So add them.
Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
Signed-off-by: Stefan Wahren <[email protected]>
---
drivers/clk/bcm/clk-raspberrypi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 79cbf0c0b401..b3b629516182 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -220,7 +220,7 @@ static int raspberrypi_fw_set_rate(struct clk_hw *hw, unsigned long rate,
ret = raspberrypi_clock_property(rpi->firmware, data,
RPI_FIRMWARE_SET_CLOCK_RATE, &_rate);
if (ret)
- dev_err_ratelimited(rpi->dev, "Failed to change %s frequency: %d",
+ dev_err_ratelimited(rpi->dev, "Failed to change %s frequency: %d\n",
clk_hw_get_name(hw), ret);
return ret;
@@ -288,7 +288,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
&min_rate);
if (ret) {
- dev_err(rpi->dev, "Failed to get clock %d min freq: %d",
+ dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
id, ret);
return ERR_PTR(ret);
}
@@ -365,7 +365,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
struct raspberrypi_clk_variant *variant;
if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) {
- dev_err(rpi->dev, "Unknown clock id: %u", clks->id);
+ dev_err(rpi->dev, "Unknown clock id: %u\n", clks->id);
return -EINVAL;
}
--
2.25.1
The clock id limit will be extended in the future, so it would be
helpful to see the actual clock id limit in case the firmware
response has been rejected.
Signed-off-by: Stefan Wahren <[email protected]>
---
drivers/clk/bcm/clk-raspberrypi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index b3b629516182..676e5379ebce 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -365,7 +365,8 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
struct raspberrypi_clk_variant *variant;
if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) {
- dev_err(rpi->dev, "Unknown clock id: %u\n", clks->id);
+ dev_err(rpi->dev, "Unknown clock id: %u (max: %u)\n",
+ clks->id, RPI_FIRMWARE_NUM_CLK_ID);
return -EINVAL;
}
--
2.25.1
On 7/13/22 08:49, Stefan Wahren wrote:
> Some log messages lacks the final newline. So add them.
>
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Signed-off-by: Stefan Wahren <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
On 7/13/22 08:49, Stefan Wahren wrote:
> The clock id limit will be extended in the future, so it would be
> helpful to see the actual clock id limit in case the firmware
> response has been rejected.
>
> Signed-off-by: Stefan Wahren <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
On 7/13/22 08:49, Stefan Wahren wrote:
> The while loop in raspberrypi_discover_clocks() relies on the assumption
> that the id of the last clock element is zero. Because this data comes
> from the Videocore firmware and it doesn't guarantuee such a behavior
> this could lead to out-of-bounds access. So fix this by providing
> a sentinel element.
>
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Link: https://github.com/raspberrypi/firmware/issues/1688
> Suggested-by: Phil Elwell <[email protected]>
> Signed-off-by: Stefan Wahren <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
Hi,
On 07-13 17:49, Stefan Wahren wrote:
>
> This series tries to fix and improvement the Raspberry Pi firmware clock
> driver. This mostly focus on clock discovery mechanism.
>
> Just a note patch #3 depends on patch #2.
>
> Stefan Wahren (3):
> clk: bcm: rpi: Prevent out-of-bounds access
> clk: bcm: rpi: Add missing newline
> clk: bcm: rpi: Show clock id limit in error case
>
Maybe is little bit late, but still :-)
Reviewed-by: Ivan T. Ivanov <[email protected]>
Thank you!
Ivan
On 7/25/22 01:18, Ivan T. Ivanov wrote:
> Hi,
>
> On 07-13 17:49, Stefan Wahren wrote:
>>
>> This series tries to fix and improvement the Raspberry Pi firmware clock
>> driver. This mostly focus on clock discovery mechanism.
>>
>> Just a note patch #3 depends on patch #2.
>>
>> Stefan Wahren (3):
>> clk: bcm: rpi: Prevent out-of-bounds access
>> clk: bcm: rpi: Add missing newline
>> clk: bcm: rpi: Show clock id limit in error case
>>
>
> Maybe is little bit late, but still :-)
>
> Reviewed-by: Ivan T. Ivanov <[email protected]>
Stephen, can you apply those patches? Thanks!
--
Florian
Hi Stephen,
Am 04.08.22 um 19:58 schrieb Florian Fainelli:
> On 7/25/22 01:18, Ivan T. Ivanov wrote:
>> Hi,
>>
>> On 07-13 17:49, Stefan Wahren wrote:
>>> This series tries to fix and improvement the Raspberry Pi firmware clock
>>> driver. This mostly focus on clock discovery mechanism.
>>>
>>> Just a note patch #3 depends on patch #2.
>>>
>>> Stefan Wahren (3):
>>> clk: bcm: rpi: Prevent out-of-bounds access
>>> clk: bcm: rpi: Add missing newline
>>> clk: bcm: rpi: Show clock id limit in error case
>>>
>> Maybe is little bit late, but still :-)
>>
>> Reviewed-by: Ivan T. Ivanov <[email protected]>
> Stephen, can you apply those patches? Thanks!
should i resend this series?
Best regards
Quoting Stefan Wahren (2022-07-13 08:49:51)
> The while loop in raspberrypi_discover_clocks() relies on the assumption
> that the id of the last clock element is zero. Because this data comes
> from the Videocore firmware and it doesn't guarantuee such a behavior
> this could lead to out-of-bounds access. So fix this by providing
> a sentinel element.
>
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Link: https://github.com/raspberrypi/firmware/issues/1688
> Suggested-by: Phil Elwell <[email protected]>
> Signed-off-by: Stefan Wahren <[email protected]>
> ---
Applied to clk-fixes
Quoting Stefan Wahren (2022-07-13 08:49:53)
> The clock id limit will be extended in the future, so it would be
> helpful to see the actual clock id limit in case the firmware
> response has been rejected.
>
> Signed-off-by: Stefan Wahren <[email protected]>
> ---
Applied to clk-fixes
Quoting Stefan Wahren (2022-07-13 08:49:52)
> Some log messages lacks the final newline. So add them.
>
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Signed-off-by: Stefan Wahren <[email protected]>
> ---
Applied to clk-fixes