mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values
@ 2026-09-28 21:19 Kyle Hendry via B4 Relay
  2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Kyle Hendry via B4 Relay @ 2026-09-28 21:19 UTC (permalink / raw)
  To: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar
  Cc: linux-renesas-soc, netdev, linux-kernel, Kyle Hendry

This series addresses issues found when reviewing another fix:
https://lore.kernel.org/netdev/20260915-rzn1-miic-fix-array-v5-1-b7173fd5b97d@reliablecontrols.com/

Invalid values from the dtb could cause out of bounds array access. Checking
the values as they're parsed should prevent this.

Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
---
Changes in v2:
- Simplify fixes by making miic_port_max the last documented port number
- Add fixes tags
- Link to v1: https://lore.kernel.org/r/20260925-miic-validate-dtb-v1-0-3a6db9bb75ec@reliablecontrols.com

---
Kyle Hendry (3):
      net: pcs: rzn1-miic: Fix port numbering on RZ/T2H
      net: pcs: rzn1-miic: Fix miic register initialization loop
      net: pcs: rzn1-miic: Validate dtb configuration values

 drivers/net/pcs/pcs-rzn1-miic.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)
---
base-commit: 014d795c73837ea2339a4ea8e8f82c6e959b845d
change-id: 20260924-miic-validate-dtb-bfc4b380b782

Best regards,
-- 
Kyle Hendry <khendry@reliablecontrols.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H
  2026-09-28 21:19 [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
@ 2026-09-28 21:19 ` Kyle Hendry via B4 Relay
  2026-10-01 11:27   ` Lad, Prabhakar
  2026-10-01 12:20   ` netdev-bot+sashiko
  2026-09-28 21:19 ` [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop Kyle Hendry via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Kyle Hendry via B4 Relay @ 2026-09-28 21:19 UTC (permalink / raw)
  To: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar
  Cc: linux-renesas-soc, netdev, linux-kernel, Kyle Hendry

From: Kyle Hendry <khendry@reliablecontrols.com>

The port numbers on the RZ/T2H are documented as 0 to 3. Update the info
structure to reflect this.

Fixes: 08f89e42121d421b ("net: pcs: rzn1-miic: Add RZ/T2H MIIC support")
Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
---
 drivers/net/pcs/pcs-rzn1-miic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
index cb74861e823c..df70309c2d97 100644
--- a/drivers/net/pcs/pcs-rzn1-miic.c
+++ b/drivers/net/pcs/pcs-rzn1-miic.c
@@ -838,7 +838,7 @@ static struct miic_of_data rzt2h_miic_of_data = {
 	.index_to_string = rzt2h_index_to_string,
 	.index_to_string_count = ARRAY_SIZE(rzt2h_index_to_string),
 	.miic_port_start = 0,
-	.miic_port_max = 4,
+	.miic_port_max = 3,
 	.sw_mode_mask = GENMASK(2, 0),
 	.reset_ids = rzt2h_reset_ids,
 	.reset_count = ARRAY_SIZE(rzt2h_reset_ids),

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop
  2026-09-28 21:19 [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
  2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
@ 2026-09-28 21:19 ` Kyle Hendry via B4 Relay
  2026-10-01 12:21   ` netdev-bot+sashiko
  2026-09-28 21:19 ` [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
  2026-09-28 21:25 ` [PATCH net-next v2 0/3] " netdev-bot+sinfo
  3 siblings, 1 reply; 11+ messages in thread
From: Kyle Hendry via B4 Relay @ 2026-09-28 21:19 UTC (permalink / raw)
  To: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar
  Cc: linux-renesas-soc, netdev, linux-kernel, Kyle Hendry

From: Kyle Hendry <khendry@reliablecontrols.com>

The function to write default values to miic registers was looping from
zero to miic_port_max, which might not cover all ports depending on SoC
numbering. Determine the port count from SoC data and loop over that.

Fixes: c112520de041758e ("net: pcs: rzn1-miic: move port range handling into SoC data")
Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
---
 drivers/net/pcs/pcs-rzn1-miic.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
index df70309c2d97..9e830932ce49 100644
--- a/drivers/net/pcs/pcs-rzn1-miic.c
+++ b/drivers/net/pcs/pcs-rzn1-miic.c
@@ -520,6 +520,7 @@ EXPORT_SYMBOL(miic_destroy);
 static int miic_init_hw(struct miic *miic, u32 cfg_mode)
 {
 	u8 sw_mode_mask = miic->of_data->sw_mode_mask;
+	int num_ports;
 	int port;
 
 	/* Unlock write access to accessory registers (cf datasheet). If this
@@ -535,7 +536,10 @@ static int miic_init_hw(struct miic *miic, u32 cfg_mode)
 	miic_reg_writel(miic, MIIC_MODCTRL,
 			((cfg_mode << __ffs(sw_mode_mask)) & sw_mode_mask));
 
-	for (port = 0; port < miic->of_data->miic_port_max; port++) {
+	num_ports = miic->of_data->miic_port_max -
+		miic->of_data->miic_port_start + 1;
+
+	for (port = 0; port < num_ports; port++) {
 		miic_converter_enable(miic, port, 0);
 		/* Disable speed/duplex control from these registers, datasheet
 		 * says switch registers should be used to setup switch port

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values
  2026-09-28 21:19 [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
  2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
  2026-09-28 21:19 ` [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop Kyle Hendry via B4 Relay
@ 2026-09-28 21:19 ` Kyle Hendry via B4 Relay
  2026-10-01 12:21   ` netdev-bot+sashiko
  2026-09-28 21:25 ` [PATCH net-next v2 0/3] " netdev-bot+sinfo
  3 siblings, 1 reply; 11+ messages in thread
From: Kyle Hendry via B4 Relay @ 2026-09-28 21:19 UTC (permalink / raw)
  To: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar
  Cc: linux-renesas-soc, netdev, linux-kernel, Kyle Hendry

From: Kyle Hendry <khendry@reliablecontrols.com>

Bad configuration values from the dtb could result in out of bounds array
access. Verify parsed values are within range for the SoC and fail the
probe if invalid.

Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
---
 drivers/net/pcs/pcs-rzn1-miic.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
index 9e830932ce49..9a4c6d3b7d42 100644
--- a/drivers/net/pcs/pcs-rzn1-miic.c
+++ b/drivers/net/pcs/pcs-rzn1-miic.c
@@ -697,9 +697,25 @@ static int miic_parse_dt(struct miic *miic, u32 *mode_cfg)
 		if (of_property_read_u32(conv, "reg", &port))
 			continue;
 
+		if (port < miic->of_data->miic_port_start ||
+		    port > miic->of_data->miic_port_max) {
+			dev_err(miic->dev, "Port number out of range: %d\n", port);
+			of_node_put(conv);
+			ret = -EINVAL;
+			goto err;
+		}
+
 		if (of_property_read_u32(conv, "renesas,miic-input", &conf))
 			continue;
 
+		if (conf >= miic->of_data->conf_to_string_count) {
+			dev_err(miic->dev, "Port %d configuration out of range: %d\n",
+				port, conf);
+			of_node_put(conv);
+			ret = -EINVAL;
+			goto err;
+		}
+
 		/* Adjust for 0 based index */
 		dt_val[port + !miic->of_data->miic_port_start] = conf;
 
@@ -709,6 +725,7 @@ static int miic_parse_dt(struct miic *miic, u32 *mode_cfg)
 	}
 
 	ret = miic_match_dt_conf(miic, dt_val, mode_cfg);
+err:
 	kfree(dt_val);
 
 	return ret;

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values
  2026-09-28 21:19 [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
                   ` (2 preceding siblings ...)
  2026-09-28 21:19 ` [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
@ 2026-09-28 21:25 ` netdev-bot+sinfo
  2026-09-29 18:13   ` Kyle Hendry
  3 siblings, 1 reply; 11+ messages in thread
From: netdev-bot+sinfo @ 2026-09-28 21:25 UTC (permalink / raw)
  To: khendry
  Cc: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar, linux-renesas-soc, netdev,
	linux-kernel

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values
  2026-09-28 21:25 ` [PATCH net-next v2 0/3] " netdev-bot+sinfo
@ 2026-09-29 18:13   ` Kyle Hendry
  0 siblings, 0 replies; 11+ messages in thread
From: Kyle Hendry @ 2026-09-29 18:13 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar, linux-renesas-soc, netdev,
	linux-kernel

On 28-Sep-26 14:25, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - Whether the issue was actually triggered, or is only theoretical
>     (e.g. found by code inspection). If it was triggered please include
>     the symptoms, like the stack trace or error messages.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

On the RZ/N1D I can trigger a KASAN slab-out-of-bounds error by defining 
miic 6 in the dts. I can also trigger a KASAN global-out-of-bounds error 
by configuring a miic input with a value outside the defined range.

I don't have a RZ/T2H to test with, but after investigating the sashiko 
reviews [1], [2] I agree that there is still a bug when checking for 
miic_port_max. If the value in rzt2h_miic_of_data is correct and being 
used as intended I can resubmit the patch using conf_conv_count instead. 
If the fixes tags were added inappropriately, I can remove them and 
resubmit this series.

[1] 
https://lore.kernel.org/all/178955899323.22033.2372714834297554596@kernel.org/
[2] 
https://lore.kernel.org/all/178955899447.22033.5754427139793967412@kernel.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H
  2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
@ 2026-10-01 11:27   ` Lad, Prabhakar
  2026-10-01 12:20   ` netdev-bot+sashiko
  1 sibling, 0 replies; 11+ messages in thread
From: Lad, Prabhakar @ 2026-10-01 11:27 UTC (permalink / raw)
  To: khendry
  Cc: Clément Léger, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lad Prabhakar, linux-renesas-soc, netdev,
	linux-kernel

Hi Kyle,

Thank you for the patch.

On Mon, Sep 28, 2026 at 10:19 PM Kyle Hendry via B4 Relay
<devnull+khendry.reliablecontrols.com@kernel.org> wrote:
>
> From: Kyle Hendry <khendry@reliablecontrols.com>
>
> The port numbers on the RZ/T2H are documented as 0 to 3. Update the info
> structure to reflect this.
>
> Fixes: 08f89e42121d421b ("net: pcs: rzn1-miic: Add RZ/T2H MIIC support")
> Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
> ---
>  drivers/net/pcs/pcs-rzn1-miic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> index cb74861e823c..df70309c2d97 100644
> --- a/drivers/net/pcs/pcs-rzn1-miic.c
> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> @@ -838,7 +838,7 @@ static struct miic_of_data rzt2h_miic_of_data = {
>         .index_to_string = rzt2h_index_to_string,
>         .index_to_string_count = ARRAY_SIZE(rzt2h_index_to_string),
>         .miic_port_start = 0,
> -       .miic_port_max = 4,
> +       .miic_port_max = 3,
The description for miic_port_max in struct miic_of_data needs
updating. I would suggest renaming this variable to miic_port_last.

Cheers,
Prabhakar

>         .sw_mode_mask = GENMASK(2, 0),
>         .reset_ids = rzt2h_reset_ids,
>         .reset_count = ARRAY_SIZE(rzt2h_reset_ids),
>
> --
> 2.43.0
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H
  2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
  2026-10-01 11:27   ` Lad, Prabhakar
@ 2026-10-01 12:20   ` netdev-bot+sashiko
  1 sibling, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 12:20 UTC (permalink / raw)
  To: khendry
  Cc: clement.leger, andrew, hkallweit1, linux, davem, edumazet, kuba,
	pabeni, prabhakar.mahadev-lad.rj, linux-renesas-soc, netdev,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] `miic_port_max` means two different things in
  drivers/net/pcs/pcs-rzn1-miic.c: - `miic_create()` (line 485) treats…

Pre-existing issues:
- [Medium] In `miic_parse_dt()` (drivers/net/pcs/pcs-rzn1-miic.c), the
  child node's DT "reg" value is used unchecked in two places: - It…

--

net: pcs: rzn1-miic: Fix port numbering on RZ/T2H

This commit changes miic_port_max in rzt2h_miic_of_data from 4 to 3. The
RZ/T2H ports are documented as 0 to 3.

> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> index cb74861e823c8..df70309c2d970 100644
> --- a/drivers/net/pcs/pcs-rzn1-miic.c
> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> @@ -838,7 +838,7 @@ static struct miic_of_data rzt2h_miic_of_data = {
>  	.index_to_string = rzt2h_index_to_string,
>  	.index_to_string_count = ARRAY_SIZE(rzt2h_index_to_string),
>  	.miic_port_start = 0,
> -	.miic_port_max = 4,
> +	.miic_port_max = 3,

[Severity: Medium]
Does this leave converter 3 out of miic_init_hw() on RZ/T2H?

This file uses miic_port_max in two ways. miic_create() treats it as the
highest valid port, inclusive:

drivers/net/pcs/pcs-rzn1-miic.c:miic_create() {
    ...
	if (port > of_data->miic_port_max || port < of_data->miic_port_start) {
    ...
}

miic_init_hw() treats it as a converter count, exclusive:

drivers/net/pcs/pcs-rzn1-miic.c:miic_init_hw() {
    ...
	for (port = 0; port < miic->of_data->miic_port_max; port++) {
		miic_converter_enable(miic, port, 0);
    ...
}

With miic_port_max = 3 and miic_port_start = 0, the loop now covers only
ports 0 to 2. miic_converter_enable() changes only the CONVRST bit for the
port it is given. So CONVRST bit 3 (MIIC_CONVRST_PHYIF_RST(3)) is no longer
cleared at probe.

CONV3 then stays in whatever reset state the bootloader left it in while
MODCTRL is reprogrammed.

A later patch in this series, "net: pcs: rzn1-miic: Fix miic register
initialization loop", changes the loop bound to
miic_port_max - miic_port_start + 1. After that change both uses agree
again.

This patch has a Fixes: tag, so it could be backported on its own. Could
the two patches be squashed or reordered, so that neither bisect nor stable
ever sees this in-between state?

>  	.sw_mode_mask = GENMASK(2, 0),
>  	.reset_ids = rzt2h_reset_ids,
>  	.reset_count = ARRAY_SIZE(rzt2h_reset_ids),

[Severity: Medium]
This isn't a bug introduced by this patch, but can miic_parse_dt() write
past the end of dt_val[]?

The DT "reg" value of each converter child is used as an index with no
bounds check:

drivers/net/pcs/pcs-rzn1-miic.c:miic_parse_dt() {
    ...
	dt_val = kmalloc_objs(*dt_val, miic->of_data->conf_conv_count);
    ...
		/* Adjust for 0 based index */
		dt_val[port + !miic->of_data->miic_port_start] = conf;
    ...
		miic_configure_phy_link(miic, conf, port, active_low);
    ...
}

On RZ/T2H conf_conv_count is 5. A child with reg >= 4 therefore writes at
index 5 or higher.

The same port value also reaches BIT(port + shift) in
miic_configure_phy_link(). A large value there would be an undefined shift.

The "renesas,miic-input" value is not range checked either. When no match
table entry matches, miic_dump_conf() uses it as
of_data->conf_to_string[conf[i]].

A later patch in this series, "net: pcs: rzn1-miic: Validate dtb
configuration values", adds port and conf range checks before the dt_val[]
write. That covers the converter child path.

The top-level "renesas,miic-switch-portin" value still looks unchecked
after that patch:

	if (of_property_read_u32(np, "renesas,miic-switch-portin", &conf) == 0)
		dt_val[0] = conf;

Could an out-of-range value there still reach conf_to_string[conf[0]] in
miic_dump_conf()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-miic-validate-dtb-v2-0-1ee3d33748e9%40reliablecontrols.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop
  2026-09-28 21:19 ` [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop Kyle Hendry via B4 Relay
@ 2026-10-01 12:21   ` netdev-bot+sashiko
  2026-10-01 12:52     ` Lad, Prabhakar
  0 siblings, 1 reply; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 12:21 UTC (permalink / raw)
  To: khendry
  Cc: clement.leger, andrew, hkallweit1, linux, davem, edumazet, kuba,
	pabeni, prabhakar.mahadev-lad.rj, linux-renesas-soc, netdev,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The Fixes tag and commit message of "net: pcs: rzn1-miic: Fix
  miic register initialization loop" blame the wrong commit and describe…

--

net: pcs: rzn1-miic: Fix miic register initialization loop

This commit changes the loop in miic_init_hw() that writes default
register values. The loop now runs for miic_port_max - miic_port_start + 1
iterations instead of miic_port_max, so the count comes from the SoC port
range.

> The function to write default values to miic registers was looping from
> zero to miic_port_max, which might not cover all ports depending on SoC
> numbering. Determine the port count from SoC data and loop over that.
>
> Fixes: c112520de041758e ("net: pcs: rzn1-miic: move port range handling into SoC data")

[Severity: Medium]
Is this Fixes tag pointing at the right commit?

Before this series, the loop in miic_init_hw() already covered every
converter on both supported SoCs:

    for (port = 0; port < miic->of_data->miic_port_max; port++) {

RZ/N1 uses miic_port_start = 1 and miic_port_max = 5, so the loop walks
ports 0..4. That is 5 converters.

RZ/T2H uses miic_port_start = 0 and miic_port_max = 4 (from 08f89e42121d),
so the loop walks ports 0..3. That is CONV0..CONV3.

c112520de041 only added the RZ/N1 data.

The loop stops covering CONV3 only after the previous patch in this
series, "net: pcs: rzn1-miic: Fix port numbering on RZ/T2H". That patch
changes rzt2h_miic_of_data.miic_port_max from 4 to 3. So this patch seems
to fix a regression from earlier in the same series, not a problem from
c112520de041.

This leaves a bisect window. At the previous patch, miic_init_hw() on
RZ/T2H runs only 3 times. The CONVRST bit for CONV3 is never cleared
during init.

The two patches also carry different Fixes tags (08f89e42121d and
c112520de041), so stable trees could pick up one without the other. If a
tree takes only this patch, RZ/T2H still has miic_port_max = 4:

    num_ports = miic->of_data->miic_port_max -
        miic->of_data->miic_port_start + 1;    /* 4 - 0 + 1 = 5 */

The loop would then clear MIIC_CONVRST_PHYIF_RST(4), which is for a
converter RZ/T2H does not have. If a tree takes only the previous patch,
CONV3 is skipped as described above.

Could the miic_port_max change and the num_ports change be squashed into
one patch with Fixes: 08f89e42121d ("net: pcs: rzn1-miic: Add RZ/T2H MIIC
support")? If not, could the commit message at least state the dependency
on the previous patch and drop the c112520de041 attribution?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-miic-validate-dtb-v2-0-1ee3d33748e9%40reliablecontrols.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values
  2026-09-28 21:19 ` [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
@ 2026-10-01 12:21   ` netdev-bot+sashiko
  0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 12:21 UTC (permalink / raw)
  To: khendry
  Cc: clement.leger, andrew, hkallweit1, linux, davem, edumazet, kuba,
	pabeni, prabhakar.mahadev-lad.rj, linux-renesas-soc, netdev,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [Low] The commit fixes a memory-safety bug but has no Fixes: tag.
- [Low] The new error message in miic_parse_dt() prints `conf`, declared
  `u32 conf;`, with `%d`: `dev_err(miic->dev, "Port %d configuration out…

--

net: pcs: rzn1-miic: Validate dtb configuration values

This commit adds range checks in miic_parse_dt() for the per-port "reg"
and "renesas,miic-input" device tree values. If either value is out of
range for the SoC, the probe fails with -EINVAL.

> Bad configuration values from the dtb could result in out of bounds array
> access. Verify parsed values are within range for the SoC and fail the
> probe if invalid.

[Severity: Low]
Should this have a Fixes: tag?

Before this patch, an unchecked "reg" value in miic_parse_dt() could
write past the end of the kmalloc'ed dt_val[] buffer. That buffer is only
conf_conv_count bytes:

	dt_val[port + !miic->of_data->miic_port_start] = conf;

The unchecked index seems to date back to the original driver. There,
dt_val[] was a stack array indexed directly by the DT "reg" value.
Would this trailer be appropriate?

Fixes: 7dc54d3b8d91 ("net: pcs: add Renesas MII converter driver")

Two earlier patches in this series already carry Fixes: tags: "net: pcs:
rzn1-miic: Fix port numbering on RZ/T2H" and "net: pcs: rzn1-miic: Fix
miic register initialization loop".

> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> index 9e830932ce49c..9a4c6d3b7d424 100644
> --- a/drivers/net/pcs/pcs-rzn1-miic.c
> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> @@ -697,9 +697,25 @@ static int miic_parse_dt(struct miic *miic, u32 *mode_cfg)
>  		if (of_property_read_u32(conv, "reg", &port))
>  			continue;
>  

[ ... ]

>  		if (of_property_read_u32(conv, "renesas,miic-input", &conf))
>  			continue;
>  
> +		if (conf >= miic->of_data->conf_to_string_count) {

[Severity: High]
This checks "renesas,miic-input", but what about the parent node's
"renesas,miic-switch-portin" value earlier in miic_parse_dt()? It is
still stored with no range check:

	if (of_property_read_u32(np, "renesas,miic-switch-portin", &conf) == 0)
		dt_val[0] = conf;

The u32 is truncated into an s8. An out-of-range value will not match
any entry in the match table. miic_match_dt_conf() then falls through to
miic_dump_conf(), which does:

	conf_name = of_data->conf_to_string[conf[i]];

Can this read a pointer from past the end of conf_to_string[] and then
pass it to dev_err() as a %s argument? Values from 128 to 254 become
negative s8 values, so for those the read would land before the start
of the array.

The binding allows only 1 or 2 on RZ/N1 and only 0 on RZ/T2H/N2H. A
value of 255 truncates to -1 (MIIC_MODCTRL_CONF_NONE) and is silently
treated as absent.

Could the same conf_to_string_count check, or the stricter per-SoC
binding values, be applied to switch-portin before it is stored in
dt_val[0]?

> +			dev_err(miic->dev, "Port %d configuration out of range: %d\n",
> +				port, conf);

[Severity: Low]
This isn't a bug, but conf is declared u32, so should this be %u? Any DT
value of 0x80000000 or above prints as negative. 0xffffffff prints as -1,
which looks like the MIIC_MODCTRL_CONF_NONE sentinel.

> +			of_node_put(conv);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-miic-validate-dtb-v2-0-1ee3d33748e9%40reliablecontrols.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop
  2026-10-01 12:21   ` netdev-bot+sashiko
@ 2026-10-01 12:52     ` Lad, Prabhakar
  0 siblings, 0 replies; 11+ messages in thread
From: Lad, Prabhakar @ 2026-10-01 12:52 UTC (permalink / raw)
  To: khendry, netdev-bot+sashiko
  Cc: clement.leger, andrew, hkallweit1, linux, davem, edumazet, kuba,
	pabeni, prabhakar.mahadev-lad.rj, linux-renesas-soc, netdev,
	linux-kernel

Hi Kyle,

On Thu, Oct 1, 2026 at 1:28 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The Fixes tag and commit message of "net: pcs: rzn1-miic: Fix
>   miic register initialization loop" blame the wrong commit and describe…
>
> --
>
> net: pcs: rzn1-miic: Fix miic register initialization loop
>
> This commit changes the loop in miic_init_hw() that writes default
> register values. The loop now runs for miic_port_max - miic_port_start + 1
> iterations instead of miic_port_max, so the count comes from the SoC port
> range.
>
> > The function to write default values to miic registers was looping from
> > zero to miic_port_max, which might not cover all ports depending on SoC
> > numbering. Determine the port count from SoC data and loop over that.
> >
> > Fixes: c112520de041758e ("net: pcs: rzn1-miic: move port range handling into SoC data")
>
> [Severity: Medium]
> Is this Fixes tag pointing at the right commit?
>
> Before this series, the loop in miic_init_hw() already covered every
> converter on both supported SoCs:
>
>     for (port = 0; port < miic->of_data->miic_port_max; port++) {
>
> RZ/N1 uses miic_port_start = 1 and miic_port_max = 5, so the loop walks
> ports 0..4. That is 5 converters.
>
> RZ/T2H uses miic_port_start = 0 and miic_port_max = 4 (from 08f89e42121d),
> so the loop walks ports 0..3. That is CONV0..CONV3.
>
> c112520de041 only added the RZ/N1 data.
>
> The loop stops covering CONV3 only after the previous patch in this
> series, "net: pcs: rzn1-miic: Fix port numbering on RZ/T2H". That patch
> changes rzt2h_miic_of_data.miic_port_max from 4 to 3. So this patch seems
> to fix a regression from earlier in the same series, not a problem from
> c112520de041.
>
> This leaves a bisect window. At the previous patch, miic_init_hw() on
> RZ/T2H runs only 3 times. The CONVRST bit for CONV3 is never cleared
> during init.
>
> The two patches also carry different Fixes tags (08f89e42121d and
> c112520de041), so stable trees could pick up one without the other. If a
> tree takes only this patch, RZ/T2H still has miic_port_max = 4:
>
>     num_ports = miic->of_data->miic_port_max -
>         miic->of_data->miic_port_start + 1;    /* 4 - 0 + 1 = 5 */
>
> The loop would then clear MIIC_CONVRST_PHYIF_RST(4), which is for a
> converter RZ/T2H does not have. If a tree takes only the previous patch,
> CONV3 is skipped as described above.
>
> Could the miic_port_max change and the num_ports change be squashed into
> one patch with Fixes: 08f89e42121d ("net: pcs: rzn1-miic: Add RZ/T2H MIIC
> support")? If not, could the commit message at least state the dependency
> on the previous patch and drop the c112520de041 attribution?
>
I totally agree with this feedback.

Cheers,
Prabhakar

> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-miic-validate-dtb-v2-0-1ee3d33748e9%40reliablecontrols.com
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-10-01 12:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-28 21:19 [PATCH net-next v2 0/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
2026-09-28 21:19 ` [PATCH net-next v2 1/3] net: pcs: rzn1-miic: Fix port numbering on RZ/T2H Kyle Hendry via B4 Relay
2026-10-01 11:27   ` Lad, Prabhakar
2026-10-01 12:20   ` netdev-bot+sashiko
2026-09-28 21:19 ` [PATCH net-next v2 2/3] net: pcs: rzn1-miic: Fix miic register initialization loop Kyle Hendry via B4 Relay
2026-10-01 12:21   ` netdev-bot+sashiko
2026-10-01 12:52     ` Lad, Prabhakar
2026-09-28 21:19 ` [PATCH net-next v2 3/3] net: pcs: rzn1-miic: Validate dtb configuration values Kyle Hendry via B4 Relay
2026-10-01 12:21   ` netdev-bot+sashiko
2026-09-28 21:25 ` [PATCH net-next v2 0/3] " netdev-bot+sinfo
2026-09-29 18:13   ` Kyle Hendry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®