Re: [PATCH net] net: dpaa: Pad packets to ETH_ZLEN

[Eric Dumazet]

On Mon, Sep 9, 2024 at 6:06 PM Sean Anderson <sean.anderson@xxxxxxxxx> wrote:
>
> When sending packets under 60 bytes, up to three bytes of the buffer following
> the data may be leaked. Avoid this by extending all packets to ETH_ZLEN,
> ensuring nothing is leaked in the padding. This bug can be reproduced by
> running
>
>         $ ping -s 11 destination
>
> Fixes: 9ad1a3749333 ("dpaa_eth: add support for DPAA Ethernet")
> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxxx>
> ---
>
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index cfe6b57b1da0..e4e8ee8b7356 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2322,6 +2322,12 @@ dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
>         }
>  #endif
>
> +       /* Packet data is always read as 32-bit words, so zero out any part of
> +        * the skb which might be sent if we have to pad the packet
> +        */
> +       if (__skb_put_padto(skb, ETH_ZLEN, false))
> +               goto enomem;
> +

This call might linearize the packet.

@nonlinear variable might be wrong after this point.

>         if (nonlinear) {
>                 /* Just create a S/G fd based on the skb */
>                 err = skb_to_sg_fd(priv, skb, &fd);
> --
> 2.35.1.1320.gc452695387.dirty
>

Perhaps this instead ?

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index cfe6b57b1da0e45613ac1bbf32ddd6ace329f4fd..5763d2f1bf8dd31b80fda0681361514dad1dc307
100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2272,12 +2272,12 @@ static netdev_tx_t
 dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
 {
        const int queue_mapping = skb_get_queue_mapping(skb);
-       bool nonlinear = skb_is_nonlinear(skb);
        struct rtnl_link_stats64 *percpu_stats;
        struct dpaa_percpu_priv *percpu_priv;
        struct netdev_queue *txq;
        struct dpaa_priv *priv;
        struct qm_fd fd;
+       bool nonlinear;
        int offset = 0;
        int err = 0;

@@ -2287,6 +2287,10 @@ dpaa_start_xmit(struct sk_buff *skb, struct
net_device *net_dev)

        qm_fd_clear_fd(&fd);

+       if (__skb_put_padto(skb, ETH_ZLEN, false))
+               goto enomem;
+
+       nonlinear = skb_is_nonlinear(skb);
        if (!nonlinear) {
                /* We're going to store the skb backpointer at the beginning
                 * of the data buffer, so we need a privately owned skb