[PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

This patch contains a fix for addr validation in fbnic_mdio_write_pcs().

Cc: stable@vger.kernel.org
Fixes: d0ce9fd7eae0 ("fbnic: Add SW shim for MDIO interface to PMD and PCS")
Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic_mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
index 709041f7fc43..d6a124889f52 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
@@ -125,7 +125,7 @@ fbnic_mdio_write_pcs(struct fbnic_dev *fbd, int addr, int regnum, u16 val)
 		addr, regnum, val);
 
 	/* Allow access to both halves of PCS for 50R2 config */
-	if (addr > 2)
+	if (addr >= 2)
 		return;
 
 	/* Skip write for reserved registers */
-- 
2.43.0

Re: [PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[Simon Horman]

On Wed, Apr 29, 2026 at 11:00:49AM -0400, mike.marciniszyn@gmail.com wrote:
> From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> 
> This patch contains a fix for addr validation in fbnic_mdio_write_pcs().

Hi Mike,

I think this warrants a bit more explanation: Why should addr 2 be
accepted? What happens from a user-perspective when it is not?

> 
> Cc: stable@vger.kernel.org
> Fixes: d0ce9fd7eae0 ("fbnic: Add SW shim for MDIO interface to PMD and PCS")
> Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>

...

-- 
pw-bot: changes-requested

Re: [PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[Mike Marciniszyn]

On Fri, May 01, 2026 at 02:46:36PM +0100, Simon Horman wrote:
> On Wed, Apr 29, 2026 at 11:00:49AM -0400, mike.marciniszyn@gmail.com wrote:
> > From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> >
> > This patch contains a fix for addr validation in fbnic_mdio_write_pcs().
>
> Hi Mike,
>
> I think this warrants a bit more explanation: Why should addr 2 be
> accepted? What happens from a user-perspective when it is not?
>

The DW IP part has two distinct PCS address ranges cooresponding
to the C45 PCS registers.

The shim translates the PCS mmd/addr/regno into specific CSR writes
to one of two zero-relative addr values into one of those two
ranges.

This patch fixes a one off in the test that could allow an invalid
CSR write if an addr == 2 was called.

I can update the commit message to reflect the above?

Mike

Re: [PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[Andrew Lunn]

On Sat, May 02, 2026 at 05:45:08AM -0400, Mike Marciniszyn wrote:
> On Fri, May 01, 2026 at 02:46:36PM +0100, Simon Horman wrote:
> > On Wed, Apr 29, 2026 at 11:00:49AM -0400, mike.marciniszyn@gmail.com wrote:
> > > From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> > >
> > > This patch contains a fix for addr validation in fbnic_mdio_write_pcs().
> >
> > Hi Mike,
> >
> > I think this warrants a bit more explanation: Why should addr 2 be
> > accepted? What happens from a user-perspective when it is not?
> >
> 
> The DW IP part has two distinct PCS address ranges cooresponding
> to the C45 PCS registers.
> 
> The shim translates the PCS mmd/addr/regno into specific CSR writes
> to one of two zero-relative addr values into one of those two
> ranges.
> 
> This patch fixes a one off in the test that could allow an invalid
> CSR write if an addr == 2 was called.

Stable runs say:

It must either fix a real bug that bothers people, ...

Can this bug be triggered with the current driver? Are there any
noticeable effects? How would somebody inside Meta know they need this
fix? This should be included in the commit message.

    Andrew

Re: [PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[Mike Marciniszyn]

On Sat, May 02, 2026 at 04:02:04PM +0200, Andrew Lunn wrote:
> On Sat, May 02, 2026 at 05:45:08AM -0400, Mike Marciniszyn wrote:
> > On Fri, May 01, 2026 at 02:46:36PM +0100, Simon Horman wrote:
> > > On Wed, Apr 29, 2026 at 11:00:49AM -0400, mike.marciniszyn@gmail.com wrote:
> > > > From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> > > >
> > > > This patch contains a fix for addr validation in fbnic_mdio_write_pcs().
> > >
> > > Hi Mike,
> > >
> > > I think this warrants a bit more explanation: Why should addr 2 be
> > > accepted? What happens from a user-perspective when it is not?
> > >
> >
> > The DW IP part has two distinct PCS address ranges cooresponding
> > to the C45 PCS registers.
> >
> > The shim translates the PCS mmd/addr/regno into specific CSR writes
> > to one of two zero-relative addr values into one of those two
> > ranges.
> >
> > This patch fixes a one off in the test that could allow an invalid
> > CSR write if an addr == 2 was called.
>
> Stable runs say:
>
> It must either fix a real bug that bothers people, ...
>
> Can this bug be triggered with the current driver? Are there any
> noticeable effects? How would somebody inside Meta know they need this
> fix? This should be included in the commit message.
>
>     Andrew

Thanks Andrew!

I am working inside Meta with Alex and Kuba.   I noticed the one off when
doing the patch that reworks the shim.

As to a real impact, that depends on the part2 series, but before that
series no one would care, which is why I had in as part of
the patch 1 series.

Without the follow on work, I suspect that no one cares or would
see any issue as I have yet to present the xpcs changes in part2.

Perhaps the best thing to do is beef up the commit and remove the
stable Cc, leaving the Fixes linkage?

Mike

Re: [PATCH net] net: eth: fbnic: Fix addr validation in pcs write

[Andrew Lunn]

> I am working inside Meta with Alex and Kuba.   I noticed the one off when
> doing the patch that reworks the shim.
> 
> As to a real impact, that depends on the part2 series, but before that
> series no one would care, which is why I had in as part of
> the patch 1 series.
> 
> Without the follow on work, I suspect that no one cares or would
> see any issue as I have yet to present the xpcs changes in part2.
> 
> Perhaps the best thing to do is beef up the commit and remove the
> stable Cc, leaving the Fixes linkage?

I'm not even sure you need the Fixes, if you say nothing is observable
broken. Just make it part of the patchset, for net-next, on going
development work.

	Andrew