* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-02 18:37 ` [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb Kyle Hendry via B4 Relay
@ 2026-09-02 18:50 ` Andrew Lunn
2026-09-14 16:37 ` Kyle Hendry
2026-09-02 22:12 ` Jakub Kicinski
2026-09-07 19:39 ` Lad, Prabhakar
2 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2026-09-02 18:50 UTC (permalink / raw)
To: khendry
Cc: Clément Léger, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
On Wed, Sep 02, 2026 at 11:37:36AM -0700, Kyle Hendry via B4 Relay wrote:
> From: Kyle Hendry <khendry@reliablecontrols.com>
>
> Add check to make sure port number is in range before writing
> to dt_val array
>
> Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
> ---
> drivers/net/pcs/pcs-rzn1-miic.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> index c50b65d064fa..3ee40eb9d545 100644
> --- a/drivers/net/pcs/pcs-rzn1-miic.c
> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> @@ -692,6 +692,12 @@ 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);
> + kfree(dt_val);
> + return -EINVAL;
> + }
> +
While reviewing this, i noticed:
/* Adjust for 0 based index */
dt_val[port + !miic->of_data->miic_port_start] = conf;
Is the ! correct?
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-02 18:50 ` Andrew Lunn
@ 2026-09-14 16:37 ` Kyle Hendry
2026-09-14 17:08 ` Andrew Lunn
0 siblings, 1 reply; 11+ messages in thread
From: Kyle Hendry @ 2026-09-14 16:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: Clément Léger, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
On 02-Sep-26 11:50, Andrew Lunn wrote:
> On Wed, Sep 02, 2026 at 11:37:36AM -0700, Kyle Hendry via B4 Relay wrote:
>> From: Kyle Hendry <khendry@reliablecontrols.com>
>>
>> Add check to make sure port number is in range before writing
>> to dt_val array
>>
>> Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
>> ---
>> drivers/net/pcs/pcs-rzn1-miic.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
>> index c50b65d064fa..3ee40eb9d545 100644
>> --- a/drivers/net/pcs/pcs-rzn1-miic.c
>> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
>> @@ -692,6 +692,12 @@ 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);
>> + kfree(dt_val);
>> + return -EINVAL;
>> + }
>> +
>
> While reviewing this, i noticed:
>
> /* Adjust for 0 based index */
> dt_val[port + !miic->of_data->miic_port_start] = conf;
>
> Is the ! correct?
>
> Andrew
Yes, this is correct because dt_val[1] is where the port config entries
start.
Depending on the SoC, the port numbering might be 0 or 1 based, so this
prevents the input config entry at dt_val[0] from being overwritten. The
logic works out to the same as:
index = port + 1 - miic->of_data->miic_port_start
-Kyle
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-14 16:37 ` Kyle Hendry
@ 2026-09-14 17:08 ` Andrew Lunn
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2026-09-14 17:08 UTC (permalink / raw)
To: Kyle Hendry
Cc: Clément Léger, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
> > While reviewing this, i noticed:
> >
> > /* Adjust for 0 based index */
> > dt_val[port + !miic->of_data->miic_port_start] = conf;
> >
> > Is the ! correct?
> >
> > Andrew
>
> Yes, this is correct because dt_val[1] is where the port config entries
> start.
>
> Depending on the SoC, the port numbering might be 0 or 1 based, so this
> prevents the input config entry at dt_val[0] from being overwritten. The
> logic works out to the same as:
>
> index = port + 1 - miic->of_data->miic_port_start
It is unusual to see a ! used like this in the middle of an
addition. Sometimes the more verbose form is less likely to get
questioned asked...
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-02 18:37 ` [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb Kyle Hendry via B4 Relay
2026-09-02 18:50 ` Andrew Lunn
@ 2026-09-02 22:12 ` Jakub Kicinski
2026-09-07 19:39 ` Lad, Prabhakar
2 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2026-09-02 22:12 UTC (permalink / raw)
To: Kyle Hendry via B4 Relay
Cc: khendry, Clément Léger, Andrew Lunn, Heiner Kallweit,
Russell King, David S. Miller, Eric Dumazet, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
On Wed, 02 Sep 2026 11:37:36 -0700 Kyle Hendry via B4 Relay wrote:
> + if (port < miic->of_data->miic_port_start || port > miic->of_data->miic_port_max) {
Please wrap this at 80 chars, some for previous patch
> + dev_err(miic->dev, "Port number out of range: %d\n", port);
> + kfree(dt_val);
> + return -EINVAL;
coccicheck says:
drivers/net/pcs/pcs-rzn1-miic.c:691:1-33: WARNING: Function "for_each_available_child_of_node" should have of_node_put() before return around line 698.
> + }
--
pw-bot: cr
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-02 18:37 ` [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb Kyle Hendry via B4 Relay
2026-09-02 18:50 ` Andrew Lunn
2026-09-02 22:12 ` Jakub Kicinski
@ 2026-09-07 19:39 ` Lad, Prabhakar
2026-09-07 20:49 ` Andrew Lunn
2 siblings, 1 reply; 11+ messages in thread
From: Lad, Prabhakar @ 2026-09-07 19:39 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 Wed, Sep 2, 2026 at 7:55 PM Kyle Hendry via B4 Relay
<devnull+khendry.reliablecontrols.com@kernel.org> wrote:
>
> From: Kyle Hendry <khendry@reliablecontrols.com>
>
> Add check to make sure port number is in range before writing
> to dt_val array
>
> Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
> ---
> drivers/net/pcs/pcs-rzn1-miic.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> index c50b65d064fa..3ee40eb9d545 100644
> --- a/drivers/net/pcs/pcs-rzn1-miic.c
> +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> @@ -692,6 +692,12 @@ 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);
> + kfree(dt_val);
> + return -EINVAL;
> + }
> +
Query to maintainers, what policy should be followed? Since the
dtbinding check complains if the port is out of range should the code
also check it?
For the patch,
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cheers,
Prabhakar
> if (of_property_read_u32(conv, "renesas,miic-input", &conf))
> continue;
>
>
> --
> 2.43.0
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-07 19:39 ` Lad, Prabhakar
@ 2026-09-07 20:49 ` Andrew Lunn
2026-09-07 21:23 ` Lad, Prabhakar
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2026-09-07 20:49 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: khendry, Clément Léger, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
On Mon, Sep 07, 2026 at 08:39:27PM +0100, Lad, Prabhakar wrote:
> Hi Kyle,
>
> Thank you for the patch.
>
> On Wed, Sep 2, 2026 at 7:55 PM Kyle Hendry via B4 Relay
> <devnull+khendry.reliablecontrols.com@kernel.org> wrote:
> >
> > From: Kyle Hendry <khendry@reliablecontrols.com>
> >
> > Add check to make sure port number is in range before writing
> > to dt_val array
> >
> > Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
> > ---
> > drivers/net/pcs/pcs-rzn1-miic.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> > index c50b65d064fa..3ee40eb9d545 100644
> > --- a/drivers/net/pcs/pcs-rzn1-miic.c
> > +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> > @@ -692,6 +692,12 @@ 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);
> > + kfree(dt_val);
> > + return -EINVAL;
> > + }
> > +
> Query to maintainers, what policy should be followed? Since the
> dtbinding check complains if the port is out of range should the code
> also check it?
Yes, the kernel should check it. Running the checker is
optional. Crashing the kernel because of an invalid DT blob is not
something you want to allow.
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/2] net: pcs: rzn1-miic: Verify port number from dtb
2026-09-07 20:49 ` Andrew Lunn
@ 2026-09-07 21:23 ` Lad, Prabhakar
0 siblings, 0 replies; 11+ messages in thread
From: Lad, Prabhakar @ 2026-09-07 21:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: khendry, Clément Léger, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lad Prabhakar, linux-renesas-soc, netdev, linux-kernel
Hi Andrew,
On Mon, Sep 7, 2026 at 9:49 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Sep 07, 2026 at 08:39:27PM +0100, Lad, Prabhakar wrote:
> > Hi Kyle,
> >
> > Thank you for the patch.
> >
> > On Wed, Sep 2, 2026 at 7:55 PM Kyle Hendry via B4 Relay
> > <devnull+khendry.reliablecontrols.com@kernel.org> wrote:
> > >
> > > From: Kyle Hendry <khendry@reliablecontrols.com>
> > >
> > > Add check to make sure port number is in range before writing
> > > to dt_val array
> > >
> > > Signed-off-by: Kyle Hendry <khendry@reliablecontrols.com>
> > > ---
> > > drivers/net/pcs/pcs-rzn1-miic.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/net/pcs/pcs-rzn1-miic.c b/drivers/net/pcs/pcs-rzn1-miic.c
> > > index c50b65d064fa..3ee40eb9d545 100644
> > > --- a/drivers/net/pcs/pcs-rzn1-miic.c
> > > +++ b/drivers/net/pcs/pcs-rzn1-miic.c
> > > @@ -692,6 +692,12 @@ 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);
> > > + kfree(dt_val);
> > > + return -EINVAL;
> > > + }
> > > +
> > Query to maintainers, what policy should be followed? Since the
> > dtbinding check complains if the port is out of range should the code
> > also check it?
>
> Yes, the kernel should check it. Running the checker is
> optional. Crashing the kernel because of an invalid DT blob is not
> something you want to allow.
>
Ack, I'll make a note of it.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 11+ messages in thread