[PATCH net v3 0/2] net: usb: asix: ax88772: Fix potential string cut

[PATCH net v3 0/2] net: usb: asix: ax88772: Fix potential string cut

[Andy Shevchenko]

The agreement and also PHY_MAX_ADDR limit suggest that the PHY address
can't occupy more than two hex digits. In some cases GCC complains about
potential string cut. In course of fixing this, introduce the PHY_ID_SIZE
predefined constant to make it easier for the users to know the bare
minimum for the buffer that holds PHY ID string (patch 1). With that,
fix the ASIX driver that triggers GCC accordingly (patch 2).

In v3:
- dropped format specifier changes (Russell, LKP)
- added predefined constant for a minimum buffer size (Russell)
- updated error message to refer to the address and not ID string (Russell)
- changed type of phy_addr to u8, otherwise GCC can't cope with its range

In v2:
- added first patch
- added a conditional to the ASIX driver (Andrew)

Andy Shevchenko (2):
  net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string
  net: usb: asix: ax88772: Increase phy_name size

 drivers/net/usb/ax88172a.c | 12 ++++++++----
 include/linux/phy.h        |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.47.2

[PATCH net v3 1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string

[Andy Shevchenko]

The PHY_ID_FMT defines the format specifier "%s:%02x" to form
the PHY ID string, where the maximum of the first part is defined
in MII_BUS_ID_SIZE, including NUL terminator, and the second part
is implied to be 3 as the maximum address is limited to 32, meaning
that 2 hex digits is more than enough, plus ':' (colon) delimiter.
However, some drivers, which are using PHY_ID_FMT, customise buffer
size and do that incorrectly. Introduce a new constant PHY_ID_SIZE
that makes the minimum required size explicit, so drivers are
encouraged to use it.

Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 19f076a71f94..5bb8dfb3d15c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -310,6 +310,7 @@ static inline long rgmii_clock(int speed)
 
 /* Used when trying to connect to a specific phy (mii bus id:phy device id) */
 #define PHY_ID_FMT "%s:%02x"
+#define PHY_ID_SIZE	(MII_BUS_ID_SIZE + 3)
 
 #define MII_BUS_ID_SIZE	61
 
-- 
2.47.2

Re: [PATCH net v3 1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string

[Russell King (Oracle)]

On Mon, Mar 24, 2025 at 04:39:29PM +0200, Andy Shevchenko wrote:
> The PHY_ID_FMT defines the format specifier "%s:%02x" to form
> the PHY ID string, where the maximum of the first part is defined
> in MII_BUS_ID_SIZE, including NUL terminator, and the second part
> is implied to be 3 as the maximum address is limited to 32, meaning
> that 2 hex digits is more than enough, plus ':' (colon) delimiter.
> However, some drivers, which are using PHY_ID_FMT, customise buffer
> size and do that incorrectly. Introduce a new constant PHY_ID_SIZE
> that makes the minimum required size explicit, so drivers are
> encouraged to use it.
> 
> Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v3 1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string

[Andy Shevchenko]

On Mon, Mar 24, 2025 at 03:06:22PM +0000, Russell King (Oracle) wrote:
> On Mon, Mar 24, 2025 at 04:39:29PM +0200, Andy Shevchenko wrote:
> > The PHY_ID_FMT defines the format specifier "%s:%02x" to form
> > the PHY ID string, where the maximum of the first part is defined
> > in MII_BUS_ID_SIZE, including NUL terminator, and the second part
> > is implied to be 3 as the maximum address is limited to 32, meaning
> > that 2 hex digits is more than enough, plus ':' (colon) delimiter.
> > However, some drivers, which are using PHY_ID_FMT, customise buffer
> > size and do that incorrectly. Introduce a new constant PHY_ID_SIZE
> > that makes the minimum required size explicit, so drivers are
> > encouraged to use it.
> > 
> > Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> 
> Thanks!

Thank you!

And just a bit of offtopic, can you look at
20250312194921.103004-1-andriy.shevchenko@linux.intel.com
and comment / apply?

That is one of the only few obstacles for me (and perhaps others, like CIs)
to enable CONFIG_WERROR when build with `make W=1` (implying existing defconfigs
for x86).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net v3 1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string

[Russell King (Oracle)]

On Mon, Mar 24, 2025 at 05:57:02PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 24, 2025 at 03:06:22PM +0000, Russell King (Oracle) wrote:
> > On Mon, Mar 24, 2025 at 04:39:29PM +0200, Andy Shevchenko wrote:
> > > The PHY_ID_FMT defines the format specifier "%s:%02x" to form
> > > the PHY ID string, where the maximum of the first part is defined
> > > in MII_BUS_ID_SIZE, including NUL terminator, and the second part
> > > is implied to be 3 as the maximum address is limited to 32, meaning
> > > that 2 hex digits is more than enough, plus ':' (colon) delimiter.
> > > However, some drivers, which are using PHY_ID_FMT, customise buffer
> > > size and do that incorrectly. Introduce a new constant PHY_ID_SIZE
> > > that makes the minimum required size explicit, so drivers are
> > > encouraged to use it.
> > > 
> > > Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > 
> > Thanks!
> 
> Thank you!
> 
> And just a bit of offtopic, can you look at
> 20250312194921.103004-1-andriy.shevchenko@linux.intel.com
> and comment / apply?

That needs to go into my patch system please. Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v3 1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string

[Andy Shevchenko]

On Mon, Mar 24, 2025 at 05:38:15PM +0000, Russell King (Oracle) wrote:
> On Mon, Mar 24, 2025 at 05:57:02PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 24, 2025 at 03:06:22PM +0000, Russell King (Oracle) wrote:

...

> > And just a bit of offtopic, can you look at
> > 20250312194921.103004-1-andriy.shevchenko@linux.intel.com
> > and comment / apply?
> 
> That needs to go into my patch system please. Thanks.

Ah, cool, just made it to appear there.

-- 
With Best Regards,
Andy Shevchenko

[PATCH net v3 2/2] net: usb: asix: ax88772: Increase phy_name size

[Andy Shevchenko]

GCC compiler (Debian 14.2.0-17) is not happy about printing
into a too short buffer (when build with `make W=1`):

 drivers/net/usb/ax88172a.c:311:9: note: ‘snprintf’ output between 4 and 66 bytes into a destination of size 20

Indeed, the buffer size is chosen based on some assumptions,
while in general the assigned name might not fit. Increase
the buffer size to cover the minimum required one. With that,
change snprintf() to use sizeof() instead of the hard coded
value.

While at it, make sure that the PHY address is not bigger than
the allowed maximum.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/usb/ax88172a.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index e47bb125048d..f613e4bc68c8 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -18,8 +18,8 @@
 struct ax88172a_private {
 	struct mii_bus *mdio;
 	struct phy_device *phydev;
-	char phy_name[20];
-	u16 phy_addr;
+	char phy_name[PHY_ID_SIZE];
+	u8 phy_addr;
 	u16 oldmode;
 	int use_embdphy;
 	struct asix_rx_fixup_info rx_fixup_info;
@@ -210,7 +210,11 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
 	ret = asix_read_phy_addr(dev, priv->use_embdphy);
 	if (ret < 0)
 		goto free;
-
+	if (ret >= PHY_MAX_ADDR) {
+		netdev_err(dev->net, "Invalid PHY address %#x\n", ret);
+		ret = -ENODEV;
+		goto free;
+	}
 	priv->phy_addr = ret;
 
 	ax88172a_reset_phy(dev, priv->use_embdphy);
@@ -308,7 +312,7 @@ static int ax88172a_reset(struct usbnet *dev)
 		   rx_ctl);
 
 	/* Connect to PHY */
-	snprintf(priv->phy_name, 20, PHY_ID_FMT,
+	snprintf(priv->phy_name, sizeof(priv->phy_name), PHY_ID_FMT,
 		 priv->mdio->id, priv->phy_addr);
 
 	priv->phydev = phy_connect(dev->net, priv->phy_name,
-- 
2.47.2

Re: [PATCH net v3 0/2] net: usb: asix: ax88772: Fix potential string cut

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 24 Mar 2025 16:39:28 +0200 you wrote:
> The agreement and also PHY_MAX_ADDR limit suggest that the PHY address
> can't occupy more than two hex digits. In some cases GCC complains about
> potential string cut. In course of fixing this, introduce the PHY_ID_SIZE
> predefined constant to make it easier for the users to know the bare
> minimum for the buffer that holds PHY ID string (patch 1). With that,
> fix the ASIX driver that triggers GCC accordingly (patch 2).
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] net: phy: Introduce PHY_ID_SIZE — minimum size for PHY ID string
    https://git.kernel.org/netdev/net-next/c/2c5ac026fd14
  - [net,v3,2/2] net: usb: asix: ax88772: Increase phy_name size
    https://git.kernel.org/netdev/net-next/c/61997271a5a7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html