[PATCH v2 0/2] phy: marvell: a3700-comphy: Minor fixes

[PATCH v2 0/2] phy: marvell: a3700-comphy: Minor fixes

[Mikhail Kobuk]

Changes in v2:
- Replace hardcode and ARRAY_SIZE() with definition
- Link to v1: https://lore.kernel.org/linux-phy/20240321164734.49273-1-m.kobuk@ispras.ru/

---
Mikhail Kobuk (2):
  phy: marvell: a3700-comphy: Fix out of bounds read
  phy: marvell: a3700-comphy: Fix hardcoded array size

 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 1/2] phy: marvell: a3700-comphy: Fix out of bounds read

[Mikhail Kobuk]

There is an out of bounds read access of 'gbe_phy_init_fix[fix_idx].addr'
every iteration after 'fix_idx' reaches 'ARRAY_SIZE(gbe_phy_init_fix)'.

Make sure 'gbe_phy_init[addr]' is used when all elements of
'gbe_phy_init_fix' array are handled.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 934337080c6c ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation")
Signed-off-by: Mikhail Kobuk <m.kobuk@ispras.ru>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
index 41162d7228c9..68710ad1ad70 100644
--- a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
@@ -611,11 +611,12 @@ static void comphy_gbe_phy_init(struct mvebu_a3700_comphy_lane *lane,
 		 * comparison to 3.125 Gbps values. These register values are
 		 * stored in "gbe_phy_init_fix" array.
 		 */
-		if (!is_1gbps && gbe_phy_init_fix[fix_idx].addr == addr) {
+		if (!is_1gbps &&
+		    fix_idx < ARRAY_SIZE(gbe_phy_init_fix) &&
+		    gbe_phy_init_fix[fix_idx].addr == addr) {
 			/* Use new value */
 			val = gbe_phy_init_fix[fix_idx].value;
-			if (fix_idx < ARRAY_SIZE(gbe_phy_init_fix))
-				fix_idx++;
+			fix_idx++;
 		} else {
 			val = gbe_phy_init[addr];
 		}
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 2/2] phy: marvell: a3700-comphy: Fix hardcoded array size

[Mikhail Kobuk]

Replace hardcoded 'gbe_phy_init' array size with defined value.

Fixes: 934337080c6c ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation")
Signed-off-by: Mikhail Kobuk <m.kobuk@ispras.ru>
---
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
index 68710ad1ad70..5d6dccfca1fb 100644
--- a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
@@ -43,6 +43,7 @@
 #define COMPHY_LANE_REG_DIRECT(reg)	(((reg) & 0x7FF) << 1)
 
 /* COMPHY registers */
+#define COMPHY_GBE_PHY_MAX_REGS	512
 #define COMPHY_POWER_PLL_CTRL		0x01
 #define PU_IVREF_BIT			BIT(15)
 #define PU_PLL_BIT			BIT(14)
@@ -296,7 +297,7 @@ static struct gbe_phy_init_data_fix gbe_phy_init_fix[] = {
 };
 
 /* 40M1G25 mode init data */
-static u16 gbe_phy_init[512] = {
+static u16 gbe_phy_init[COMPHY_GBE_PHY_MAX_REGS] = {
 	/* 0       1       2       3       4       5       6       7 */
 	/*-----------------------------------------------------------*/
 	/* 8       9       A       B       C       D       E       F */
@@ -603,7 +604,7 @@ static void comphy_gbe_phy_init(struct mvebu_a3700_comphy_lane *lane,
 	u16 val;
 
 	fix_idx = 0;
-	for (addr = 0; addr < 512; addr++) {
+	for (addr = 0; addr < COMPHY_GBE_PHY_MAX_REGS; addr++) {
 		/*
 		 * All PHY register values are defined in full for 3.125Gbps
 		 * SERDES speed. The values required for 1.25 Gbps are almost
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/2] phy: marvell: a3700-comphy: Fix hardcoded array size

[Miquel Raynal]

Hi Mikhail,

m.kobuk@ispras.ru wrote on Mon, 25 Mar 2024 23:12:50 +0300:

> Replace hardcoded 'gbe_phy_init' array size with defined value.
> 
> Fixes: 934337080c6c ("phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation")
> Signed-off-by: Mikhail Kobuk <m.kobuk@ispras.ru>

LGTM.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 0/2] phy: marvell: a3700-comphy: Minor fixes

[Vinod Koul]

On 25-03-24, 23:12, Mikhail Kobuk wrote:
> Changes in v2:
> - Replace hardcode and ARRAY_SIZE() with definition
> - Link to v1: https://lore.kernel.org/linux-phy/20240321164734.49273-1-m.kobuk@ispras.ru/

This does not apply for me, pls rebase on phy/fixes

> 
> ---
> Mikhail Kobuk (2):
>   phy: marvell: a3700-comphy: Fix out of bounds read
>   phy: marvell: a3700-comphy: Fix hardcoded array size
> 
>  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> -- 
> 2.44.0

-- 
~Vinod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 0/2] phy: marvell: a3700-comphy: Minor fixes

[Vinod Koul]

On Mon, 25 Mar 2024 23:12:48 +0300, Mikhail Kobuk wrote:
> Changes in v2:
> - Replace hardcode and ARRAY_SIZE() with definition
> - Link to v1: https://lore.kernel.org/linux-phy/20240321164734.49273-1-m.kobuk@ispras.ru/
> 

Applied, thanks!

[1/2] phy: marvell: a3700-comphy: Fix out of bounds read
      commit: e4308bc22b9d46cf33165c9dfaeebcf29cd56f04
[2/2] phy: marvell: a3700-comphy: Fix hardcoded array size
      commit: 627207703b73615653eea5ab7a841d5b478d961e

Best regards,
-- 
~Vinod



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy