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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Apr 2023 18:48:14 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Roman Gushchin <roman.gushchin@...ux.dev>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Rafal Ozieblo <rafalo@...ence.com>,
        Lars-Peter Clausen <lars@...afoo.de>
Subject: Re: [PATCH net] net: macb: fix a memory corruption in extended
 buffer descriptor mode

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@...ux.dev>
> Co-developed-by: Lars-Peter Clausen <lars@...afoo.de>
> Signed-off-by: Lars-Peter Clausen <lars@...afoo.de>
> ---
>  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@...inx.com> on v2.