| Message ID | 9d4496c2-e102-4fc6-afe2-5a917930d15a@kili.mountain (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: bcm: rpi: Fix off by one in raspberrypi_discover_clocks() | expand |
Hi Dan, Am 19.04.23 um 09:56 schrieb Dan Carpenter: > Smatch detected an off by one in this code: > > drivers/clk/bcm/clk-raspberrypi.c:374 raspberrypi_discover_clocks() > error: buffer overflow 'data->hws' 16 <= 16 > > The data->hws[] array has RPI_FIRMWARE_NUM_CLK_ID elements so the > > comparison needs to changed to >=. > > Fixes: 12c90f3f27bb ("clk: bcm: rpi: Add variant structure") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > The clks[] array, on the other hand, is correct. It allocates > RPI_FIRMWARE_NUM_CLK_ID + 1 elements because the last element is a > sentinal. > > drivers/clk/bcm/clk-raspberrypi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c > index eb399a4d141b..d4b43e33035c 100644 > --- a/drivers/clk/bcm/clk-raspberrypi.c > +++ b/drivers/clk/bcm/clk-raspberrypi.c > @@ -356,7 +356,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi, > while (clks->id) { > struct raspberrypi_clk_variant *variant; > > - if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) { > + if (clks->id >= RPI_FIRMWARE_NUM_CLK_ID) { > dev_err(rpi->dev, "Unknown clock id: %u (max: %u)\n", > clks->id, RPI_FIRMWARE_NUM_CLK_ID); the change looks good, but shouldn't we also reduce the max in the error message by one? > return -EINVAL;
On Wed, Apr 19, 2023 at 05:55:12PM +0200, Stefan Wahren wrote: > Hi Dan, > > Am 19.04.23 um 09:56 schrieb Dan Carpenter: > > Smatch detected an off by one in this code: > > > > drivers/clk/bcm/clk-raspberrypi.c:374 raspberrypi_discover_clocks() > > error: buffer overflow 'data->hws' 16 <= 16 > > > > The data->hws[] array has RPI_FIRMWARE_NUM_CLK_ID elements so the > > > comparison needs to changed to >=. > > > > Fixes: 12c90f3f27bb ("clk: bcm: rpi: Add variant structure") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > --- > > The clks[] array, on the other hand, is correct. It allocates > > RPI_FIRMWARE_NUM_CLK_ID + 1 elements because the last element is a > > sentinal. > > > > drivers/clk/bcm/clk-raspberrypi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c > > index eb399a4d141b..d4b43e33035c 100644 > > --- a/drivers/clk/bcm/clk-raspberrypi.c > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > @@ -356,7 +356,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi, > > while (clks->id) { > > struct raspberrypi_clk_variant *variant; > > - if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) { > > + if (clks->id >= RPI_FIRMWARE_NUM_CLK_ID) { > > dev_err(rpi->dev, "Unknown clock id: %u (max: %u)\n", > > clks->id, RPI_FIRMWARE_NUM_CLK_ID); > > the change looks good, but shouldn't we also reduce the max in the error > message by one? Sure. Let me resend. regards, dan carpenter
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index eb399a4d141b..d4b43e33035c 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -356,7 +356,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi, while (clks->id) { struct raspberrypi_clk_variant *variant; - if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) { + if (clks->id >= RPI_FIRMWARE_NUM_CLK_ID) { dev_err(rpi->dev, "Unknown clock id: %u (max: %u)\n", clks->id, RPI_FIRMWARE_NUM_CLK_ID); return -EINVAL;
Smatch detected an off by one in this code: drivers/clk/bcm/clk-raspberrypi.c:374 raspberrypi_discover_clocks() error: buffer overflow 'data->hws' 16 <= 16 The data->hws[] array has RPI_FIRMWARE_NUM_CLK_ID elements so the > comparison needs to changed to >=. Fixes: 12c90f3f27bb ("clk: bcm: rpi: Add variant structure") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- The clks[] array, on the other hand, is correct. It allocates RPI_FIRMWARE_NUM_CLK_ID + 1 elements because the last element is a sentinal. drivers/clk/bcm/clk-raspberrypi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)