net: mvneta: Avoid the misuse of the '_t' variants

[PATCH net-next v1] net: mvneta: Avoid the misuse of the '_t' variants

Posted by Yan Zhen 1 year, 3 months ago

Type conversions (especially when narrowing down types) can lead to 
unexpected behavior and should be done with extreme caution.

In this case if someone tries to set the tx_pending to 65536(0x10000),
after forcing it to convert to u16, it becomes 0x0000, they will get
the minimum supported size and not the maximum.

Based on previous discussions [1], such behavior may confuse users or even
create unexpected errors.

[1] https://lore.kernel.org/netdev/d23dfbf563714d7090d163a075ca9a51@AcuMS.aculab.com/

Signed-off-by: Yan Zhen <yanzhen@vivo.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index d72b2d5f96db..b41de182cc88 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4753,8 +4753,8 @@ mvneta_ethtool_set_ringparam(struct net_device *dev,
 	pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ?
 		ring->rx_pending : MVNETA_MAX_RXD;
 
-	pp->tx_ring_size = clamp_t(u16, ring->tx_pending,
-				   MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
+	pp->tx_ring_size = clamp(ring->tx_pending,
+				 MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
 	if (pp->tx_ring_size != ring->tx_pending)
 		netdev_warn(dev, "TX queue size set to %u (requested %u)\n",
 			    pp->tx_ring_size, ring->tx_pending);
-- 
2.34.1

Re: [PATCH net-next v1] net: mvneta: Avoid the misuse of the '_t' variants

Posted by Jakub Kicinski 1 year, 3 months ago

On Thu,  5 Sep 2024 14:36:45 +0800 Yan Zhen wrote:
> In this case if someone tries to set the tx_pending to 65536(0x10000),
> after forcing it to convert to u16, it becomes 0x0000, they will get
> the minimum supported size and not the maximum.

Core validates against tx_max_pending before calling the driver.
-- 
pw-bot: reject

RE: [PATCH net-next v1] net: mvneta: Avoid the misuse of the '_t' variants

Posted by David Laight 1 year, 3 months ago

From: Yan Zhen
> Sent: 05 September 2024 07:37
> 
> Type conversions (especially when narrowing down types) can lead to
> unexpected behavior and should be done with extreme caution.
> 
> In this case if someone tries to set the tx_pending to 65536(0x10000),
> after forcing it to convert to u16, it becomes 0x0000, they will get
> the minimum supported size and not the maximum.
> 
> Based on previous discussions [1], such behavior may confuse users or even
> create unexpected errors.
> 
> [1] https://lore.kernel.org/netdev/d23dfbf563714d7090d163a075ca9a51@AcuMS.aculab.com/
> 
> Signed-off-by: Yan Zhen <yanzhen@vivo.com>

Reviewed-by: david.laight@aculab.com

> ---
>  drivers/net/ethernet/marvell/mvneta.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index d72b2d5f96db..b41de182cc88 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4753,8 +4753,8 @@ mvneta_ethtool_set_ringparam(struct net_device *dev,
>  	pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ?
>  		ring->rx_pending : MVNETA_MAX_RXD;
> 
> -	pp->tx_ring_size = clamp_t(u16, ring->tx_pending,
> -				   MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
> +	pp->tx_ring_size = clamp(ring->tx_pending,
> +				 MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
>  	if (pp->tx_ring_size != ring->tx_pending)
>  		netdev_warn(dev, "TX queue size set to %u (requested %u)\n",
>  			    pp->tx_ring_size, ring->tx_pending);
> --
> 2.34.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)