[PATCH net] net: usb: rtl8150: Fix frame padding

[PATCH net] net: usb: rtl8150: Fix frame padding

[Michal Pecio]

TX frames aren't padded and unknown memory is sent into the ether.

Theoretically, it isn't even guaranteed that the extra memory exists
and can be sent out, which could cause further problems. In practice,
I found that plenty of tailroom exists in the skb itself (in my test
with ping at least) and skb_padto() easily succeeds, so use it here.

In the event of -ENOMEM drop the frame like other drivers do.

The use of one more padding byte instead of a USB zero-length packet
is retained to avoid regression. I have a dodgy Etron xHCI controller
which doesn't seem to support sending ZLPs at all.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/net/usb/rtl8150.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 92add3daadbb..d6dce8babae0 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -685,9 +685,14 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
 	rtl8150_t *dev = netdev_priv(netdev);
 	int count, res;
 
+	/* pad the frame and ensure terminating USB packet, datasheet 9.2.3 */
+	count = max(skb->len, ETH_ZLEN);
+	if (count % 64 == 0)
+		count++;
+	if (skb_padto(skb, count))
+		return NETDEV_TX_OK;
+
 	netif_stop_queue(netdev);
-	count = (skb->len < 60) ? 60 : skb->len;
-	count = (count & 0x3f) ? count : count + 1;
 	dev->tx_skb = skb;
 	usb_fill_bulk_urb(dev->tx_urb, dev->udev, usb_sndbulkpipe(dev->udev, 2),
 		      skb->data, count, write_bulk_callback, dev);
-- 
2.48.1

Re: [PATCH net] net: usb: rtl8150: Fix frame padding

[Simon Horman]

On Sun, Oct 12, 2025 at 10:00:42PM +0200, Michal Pecio wrote:
> TX frames aren't padded and unknown memory is sent into the ether.
> 
> Theoretically, it isn't even guaranteed that the extra memory exists
> and can be sent out, which could cause further problems. In practice,
> I found that plenty of tailroom exists in the skb itself (in my test
> with ping at least) and skb_padto() easily succeeds, so use it here.
> 
> In the event of -ENOMEM drop the frame like other drivers do.
> 
> The use of one more padding byte instead of a USB zero-length packet
> is retained to avoid regression. I have a dodgy Etron xHCI controller
> which doesn't seem to support sending ZLPs at all.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---
>  drivers/net/usb/rtl8150.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 92add3daadbb..d6dce8babae0 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -685,9 +685,14 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	int count, res;
>  
> +	/* pad the frame and ensure terminating USB packet, datasheet 9.2.3 */
> +	count = max(skb->len, ETH_ZLEN);
> +	if (count % 64 == 0)
> +		count++;
> +	if (skb_padto(skb, count))
> +

Hi Michal,

I think this should also increment a dropped counter.
As this driver already uses dev->netdev->stats [*]
I think that would be:

		dev->netdev->stats.tx_dropped++;

[*] I specifically mention this, for the record because,
    new users are discouraged. But this driver is an existing user
    so I think we are ok.

> +		return NETDEV_TX_OK;
>  	netif_stop_queue(netdev);
> -	count = (skb->len < 60) ? 60 : skb->len;
> -	count = (count & 0x3f) ? count : count + 1;
>  	dev->tx_skb = skb;
>  	usb_fill_bulk_urb(dev->tx_urb, dev->udev, usb_sndbulkpipe(dev->udev, 2),
>  		      skb->data, count, write_bulk_callback, dev);
> -- 
> 2.48.1
> 

Re: [PATCH net] net: usb: rtl8150: Fix frame padding

[Michal Pecio]

On Mon, 13 Oct 2025 10:57:51 +0100, Simon Horman wrote:
> Hi Michal,
> 
> I think this should also increment a dropped counter.
> As this driver already uses dev->netdev->stats [*]
> I think that would be:
> 
> 		dev->netdev->stats.tx_dropped++;
>
> [*] I specifically mention this, for the record because,
>     new users are discouraged. But this driver is an existing user
>     so I think we are ok.

Thanks, makes sense, will do.

I will only drop "dev->" because it's superfluous - we already have
'netdev' here, which was used to obtain 'dev' in the first place.

Re: [PATCH net] net: usb: rtl8150: Fix frame padding

[Simon Horman]

On Mon, Oct 13, 2025 at 12:50:43PM +0200, Michal Pecio wrote:
> On Mon, 13 Oct 2025 10:57:51 +0100, Simon Horman wrote:
> > Hi Michal,
> > 
> > I think this should also increment a dropped counter.
> > As this driver already uses dev->netdev->stats [*]
> > I think that would be:
> > 
> > 		dev->netdev->stats.tx_dropped++;
> >
> > [*] I specifically mention this, for the record because,
> >     new users are discouraged. But this driver is an existing user
> >     so I think we are ok.
> 
> Thanks, makes sense, will do.
> 
> I will only drop "dev->" because it's superfluous - we already have
> 'netdev' here, which was used to obtain 'dev' in the first place.

Sounds good, sorry for missing that detail.