Re: [PATCH stable v4.4 1/2] net: stmmac: use correct DMA buffer size in the RX descriptor

From: Ben Hutchings
Date: Sun Jan 26 2020 - 13:27:33 EST

On Mon, 2019-12-16 at 21:52 -0800, Aviraj CJ wrote:
> upstream 583e6361414903c5206258a30e5bd88cb03c0254 commit
>
> We always program the maximum DMA buffer size into the receive descriptor,
> although the allocated size may be less. E.g. with the default MTU size
> we allocate only 1536 bytes. If somebody sends us a bigger frame, then
> memory may get corrupted.
>
> Program DMA using exact buffer sizes.
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@xxxxxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> [acj: backport to v4.4 -stable :
> - Modified patch since v4.4 driver has no support for Big endian
> - Skipped the section modifying non-existent functions in dwmac4_descs.c and
> dwxgmac2_descs.c ]
> Signed-off-by: Aviraj CJ <acj@xxxxxxxxx>

I've queued up these two fixes for 3.16. This first patch needed a
little more adjustment as I had previously backported commit
8137b6ef0ce4 "net: stmmac: Fix RX packet size > 8191". Perhaps that
should also be applied to 4.4?

Ben.

> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
> drivers/net/ethernet/stmicro/stmmac/descs_com.h | 14 ++++++++++----
> drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 10 +++++++---
> drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 10 +++++++---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
> 5 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 623c6ed8764a..803df6a32ba9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -301,7 +301,7 @@ struct dma_features {
> struct stmmac_desc_ops {
> /* DMA RX descriptor ring initialization */
> void (*init_rx_desc) (struct dma_desc *p, int disable_rx_ic, int mode,
> - int end);
> + int end, int bfsize);
> /* DMA TX descriptor ring initialization */
> void (*init_tx_desc) (struct dma_desc *p, int mode, int end);
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/descs_com.h b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
> index 6f2cc78c5cf5..6b83fc8e6fbe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/descs_com.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
> @@ -33,9 +33,10 @@
> /* Specific functions used for Ring mode */
>
> /* Enhanced descriptors */
> -static inline void ehn_desc_rx_set_on_ring(struct dma_desc *p, int end)
> +static inline void ehn_desc_rx_set_on_ring(struct dma_desc *p, int end, int bfsize)
> {
> - p->des01.erx.buffer2_size = BUF_SIZE_8KiB - 1;
> + if (bfsize == BUF_SIZE_16KiB)
> + p->des01.erx.buffer2_size = BUF_SIZE_8KiB - 1;
> if (end)
> p->des01.erx.end_ring = 1;
> }
> @@ -61,9 +62,14 @@ static inline void enh_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
> }
>
> /* Normal descriptors */
> -static inline void ndesc_rx_set_on_ring(struct dma_desc *p, int end)
> +static inline void ndesc_rx_set_on_ring(struct dma_desc *p, int end, int bfsize)
> {
> - p->des01.rx.buffer2_size = BUF_SIZE_2KiB - 1;
> + int size;
> +
> + if (bfsize >= BUF_SIZE_2KiB) {
> + size = min(bfsize - BUF_SIZE_2KiB + 1, BUF_SIZE_2KiB - 1);
> + p->des01.rx.buffer2_size = size;
> + }
> if (end)
> p->des01.rx.end_ring = 1;
> }
> diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> index 7d944449f5ef..9ecb3a948f86 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> @@ -238,16 +238,20 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
> }
>
> static void enh_desc_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
> - int mode, int end)
> + int mode, int end, int bfsize)
> {
> + int bfsize1;
> +
> p->des01.all_flags = 0;
> p->des01.erx.own = 1;
> - p->des01.erx.buffer1_size = BUF_SIZE_8KiB - 1;
> +
> + bfsize1 = min(bfsize, BUF_SIZE_8KiB - 1);
> + p->des01.erx.buffer1_size = bfsize1;
>
> if (mode == STMMAC_CHAIN_MODE)
> ehn_desc_rx_set_on_chain(p, end);
> else
> - ehn_desc_rx_set_on_ring(p, end);
> + ehn_desc_rx_set_on_ring(p, end, bfsize);
>
> if (disable_rx_ic)
> p->des01.erx.disable_ic = 1;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> index 48c3456445b2..07e0c03cfb10 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> @@ -121,16 +121,20 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
> }
>
> static void ndesc_init_rx_desc(struct dma_desc *p, int disable_rx_ic, int mode,
> - int end)
> + int end, int bfsize)
> {
> + int bfsize1;
> +
> p->des01.all_flags = 0;
> p->des01.rx.own = 1;
> - p->des01.rx.buffer1_size = BUF_SIZE_2KiB - 1;
> +
> + bfsize1 = min(bfsize, (BUF_SIZE_2KiB - 1));
> + p->des01.rx.buffer1_size = bfsize1;
>
> if (mode == STMMAC_CHAIN_MODE)
> ndesc_rx_set_on_chain(p, end);
> else
> - ndesc_rx_set_on_ring(p, end);
> + ndesc_rx_set_on_ring(p, end, bfsize);
>
> if (disable_rx_ic)
> p->des01.rx.disable_ic = 1;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f4d6512f066c..e9d41e03121c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -964,11 +964,11 @@ static void stmmac_clear_descriptors(struct stmmac_priv *priv)
> if (priv->extend_desc)
> priv->hw->desc->init_rx_desc(&priv->dma_erx[i].basic,
> priv->use_riwt, priv->mode,
> - (i == rxsize - 1));
> + (i == rxsize - 1), priv->dma_buf_sz);
> else
> priv->hw->desc->init_rx_desc(&priv->dma_rx[i],
> priv->use_riwt, priv->mode,
> - (i == rxsize - 1));
> + (i == rxsize - 1), priv->dma_buf_sz);
> for (i = 0; i < txsize; i++)
> if (priv->extend_desc)
> priv->hw->desc->init_tx_desc(&priv->dma_etx[i].basic,
--
Ben Hutchings
The program is absolutely right; therefore, the computer must be wrong.

Attachment: signature.asc
Description: This is a digitally signed message part