phy: ocelot-serdes: fix out-of-bounds read
PATCH 1/1 phy: ocelot-serdes: fix out-of-bounds read
Currently, there is an out-of-bounds read on array ctrl->phys, once variable i reaches the maximum array size of SERDES_MAX in the for loop.
Fix this by changing the condition in the for loop from i <= SERDES_MAX to i < SERDES_MAX.
Addresses-Coverity-ID: 1473966 ("Out-of-bounds read")
Addresses-Coverity-ID: 1473959 ("Out-of-bounds read")
--- drivers/phy/mscc/phy-ocelot-serdes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/phy/mscc/phy-ocelot-serdes.c +2 -2
@@ -206,7 +206,7 @@ static struct phy *serdes_simple_xlate(struct device *dev, port = args->args[0 ]; idx = args->args[1 ]; -for (i =0 ; i <=SERDES_MAX ; i++) { +for (i =0 ; i <SERDES_MAX ; i++) {struct serdes_macro *macro =phy_get_drvdata (ctrl->phys[i]);if (idx != macro->idx)
@@ -260,7 +260,7 @@ static int serdes_probe(struct platform_device *pdev)if (!ctrl->regs)return -ENODEV ; -for (i =0 ; i <=SERDES_MAX ; i++) { +for (i =0 ; i <SERDES_MAX ; i++) { ret =serdes_phy_create (ctrl, i, &ctrl->phys[i]);if (ret)return ret;
Signature · 1 line
2.7.4
Hi Gustavo,
Quoted 7 lines · Gustavo A. R. Silva
Currently, there is an out-of-bounds read on array
ctrl->phys, once variable i reaches the maximum array size ofSERDES_MAXin the for loop.Fix this by changing the condition in the for loop from i <=
SERDES_MAXto i <SERDES_MAX.
Thanks for the heads up. However, as defined today, SERDES_MAX is a valid value so I need it in the iteration. There are two possible fixes though:
Either we let all the for loops as for (i = 0; i <= SERDES_MAX; i++) and define ctrl->phys as an array of size SERDES_MAX + 1.
Or we modify the for loops as for (i = 0; i < SERDES_MAX; i++) and we update SERDES_MAX in include/dt-bindings/phy/phy-ocelot-serdes.h to be SERDES6G_MAX + 1.
As you wish!
Thanks,
Quentin
Quoted 33 lines
Addresses-Coverity-ID: 1473966 ("Out-of-bounds read")
Addresses-Coverity-ID: 1473959 ("Out-of-bounds read")Fixes 51f6b410fc22 ("phy: add driver for Microsemi Ocelot SerDes muxing")Signed-off-by Gustavo A. R. Silva <[email protected]>--- drivers/phy/mscc/phy-ocelot-serdes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/phy/mscc/phy-ocelot-serdes.c +2
-2@@ -206,7 +206,7 @@ static struct phy *serdes_simple_xlate(struct device *dev, port = args->args[0 ]; idx = args->args[1 ]; -for (i =0 ; i <=SERDES_MAX ; i++) { +for (i =0 ; i <SERDES_MAX ; i++) {struct serdes_macro *macro =phy_get_drvdata (ctrl->phys[i]);if (idx != macro->idx)@@ -260,7 +260,7 @@ static int serdes_probe(struct platform_device *pdev)if (!ctrl->regs)return -ENODEV ; -for (i =0 ; i <=SERDES_MAX ; i++) { +for (i =0 ; i <SERDES_MAX ; i++) { ret =serdes_phy_create (ctrl, i, &ctrl->phys[i]);if (ret)return ret;Signature · 1 line
2.7.4
Hey Quentin,
Quoted 7 lines · Quentin Schulz
Hi Gustavo,
Quoted 4 lines · Gustavo A. R. Silva
Currently, there is an out-of-bounds read on array
ctrl->phys, once variable i reaches the maximum array size ofSERDES_MAXin the for loop.
Quoted 8 lines
Quoted 3 lines
Fix this by changing the condition in the for loop from i <=
SERDES_MAXto i <SERDES_MAX.Thanks for the heads up. However, as defined today,
SERDES_MAXis a valid value so I need it in the iteration. There are two possible fixes though:
Oh okay. I've got it.
Quoted 7 lines
Either we let all the for loops as
for (i = 0; i <= SERDES_MAX; i++)and definectrl->physas an array of sizeSERDES_MAX+ 1.Or we modify the for loops as
for (i = 0; i < SERDES_MAX; i++)and we updateSERDES_MAXininclude/dt-bindings/phy/phy-ocelot-serdes.hto beSERDES6G_MAX+ 1.
I think this one is better.
Quoted 2 lines
As you wish!
I'll send v2 shortly.
Thanks for the feedback. :)
--
Gustavo