Re: [PATCH net] net: macb: fix a memory corruption in extended buffer descriptor mode

From: Jakub Kicinski
Date: Tue Apr 11 2023 - 21:48:25 EST


On Fri, 7 Apr 2023 10:24:02 -0700 Roman Gushchin wrote:
> The problem is resolved by extending the MACB_RX_WADDR_SIZE
> in the extended mode.
>
> Fixes: 7b4296148066 ("net: macb: Add support for PTP timestamps in DMA descriptors")
> Signed-off-by: Roman Gushchin <roman.gushchin@xxxxxxxxx>
> Co-developed-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> ---
> drivers/net/ethernet/cadence/macb.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index c1fc91c97cee..1b330f7cfc09 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -826,8 +826,13 @@ struct macb_dma_desc_ptp {
> #define MACB_RX_USED_SIZE 1
> #define MACB_RX_WRAP_OFFSET 1
> #define MACB_RX_WRAP_SIZE 1
> +#ifdef MACB_EXT_DESC
> +#define MACB_RX_WADDR_OFFSET 3
> +#define MACB_RX_WADDR_SIZE 29
> +#else
> #define MACB_RX_WADDR_OFFSET 2
> #define MACB_RX_WADDR_SIZE 30
> +#endif

Changing register definition based on Kconfig seems a bit old school.

Where is the extended descriptor mode enabled? Is it always on if
Kconfig is set or can it be off for some platforms based on other
capabilities? Judging by macb_dma_desc_get_size() small descriptors
can still be used even with EXT_DESC?

If I'm grepping correctly thru the painful macro magic this register
is only used in macb_get_addr(). It'd seem a bit more robust to me
to open code the extraction of the address based on bp->hw_dma_cap
in that one function.

In addition to maintainers please also CC Harini Katakam
<harini.katakam@xxxxxxxxxx> on v2.