Re: [PATCH net] net: marvell: prestera: use unaligned accessors for DSA tag

[David Laight]

On Sat, 20 Jun 2026 17:37:39 +0800
Runyu Xiao <runyu.xiao@xxxxxxxxxx> wrote:

> Prestera parses and builds its 16-byte DSA tag from an skb byte buffer.
> The current code casts the tag pointer to __be32 * and then reads or
> writes the four tag words through that typed pointer.
> 
> The tag pointer is derived from skb data, but that only identifies the
> protocol tag location inside the packet buffer. It does not make the tag
> a naturally aligned __be32 array. Use the unaligned big-endian helpers
> for both parsing and building the tag.
> 
> This issue was detected by our static analysis tool and confirmed by
> manual audit. The same access pattern was validated with UBSAN alignment
> instrumentation by keeping the original cast from a u8 DSA tag buffer to
> __be32 * and reading dsa_words[i] from a deliberately misaligned tag
> buffer. UBSAN reported misaligned-access loads of type '__be32' in
> prestera_dsa_parse().
> 
> The driver has the same source-level issue: the RX path parses bytes at
> skb->data - ETH_TLEN, and the TX path writes the tag at skb->data +
> 2 * ETH_ALEN. Those offsets identify the DSA tag bytes, but they do not
> establish a __be32 object or a 4-byte alignment guarantee for typed loads
> and stores.

Stop sending these 'fixes' unless you can do proper analysis.
skb data is guaranteed to be aligned so that these reads (and ones of
the IP/TCP/UDP headers) are aligned.

	David


> 
> Fixes: 501ef3066c89 ("net: marvell: prestera: Add driver for Prestera family ASIC devices")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>  .../ethernet/marvell/prestera/prestera_dsa.c  | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_dsa.c b/drivers/net/ethernet/marvell/prestera/prestera_dsa.c
> index b7e89c0ca5c0..276f98cbd50e 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_dsa.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_dsa.c
> @@ -4,6 +4,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/errno.h>
> +#include <linux/unaligned.h>
>  #include <linux/string.h>
>  
>  #include "prestera_dsa.h"
> @@ -33,15 +34,14 @@
>  
>  int prestera_dsa_parse(struct prestera_dsa *dsa, const u8 *dsa_buf)
>  {
> -	__be32 *dsa_words = (__be32 *)dsa_buf;
>  	enum prestera_dsa_cmd cmd;
>  	u32 words[4];
>  	u32 field;
>  
> -	words[0] = ntohl(dsa_words[0]);
> -	words[1] = ntohl(dsa_words[1]);
> -	words[2] = ntohl(dsa_words[2]);
> -	words[3] = ntohl(dsa_words[3]);
> +	words[0] = get_unaligned_be32(dsa_buf);
> +	words[1] = get_unaligned_be32(dsa_buf + 4);
> +	words[2] = get_unaligned_be32(dsa_buf + 8);
> +	words[3] = get_unaligned_be32(dsa_buf + 12);
>  
>  	/* set the common parameters */
>  	cmd = (enum prestera_dsa_cmd)FIELD_GET(PRESTERA_DSA_W0_CMD, words[0]);
> @@ -82,7 +82,6 @@ int prestera_dsa_parse(struct prestera_dsa *dsa, const u8 *dsa_buf)
>  
>  int prestera_dsa_build(const struct prestera_dsa *dsa, u8 *dsa_buf)
>  {
> -	__be32 *dsa_words = (__be32 *)dsa_buf;
>  	u32 dev_num = dsa->hw_dev_num;
>  	u32 words[4] = { 0 };
>  
> @@ -98,10 +97,10 @@ int prestera_dsa_build(const struct prestera_dsa *dsa, u8 *dsa_buf)
>  	words[1] |= FIELD_PREP(PRESTERA_DSA_W1_EXT_BIT, 1);
>  	words[2] |= FIELD_PREP(PRESTERA_DSA_W2_EXT_BIT, 1);
>  
> -	dsa_words[0] = htonl(words[0]);
> -	dsa_words[1] = htonl(words[1]);
> -	dsa_words[2] = htonl(words[2]);
> -	dsa_words[3] = htonl(words[3]);
> +	put_unaligned_be32(words[0], dsa_buf);
> +	put_unaligned_be32(words[1], dsa_buf + 4);
> +	put_unaligned_be32(words[2], dsa_buf + 8);
> +	put_unaligned_be32(words[3], dsa_buf + 12);
>  
>  	return 0;
>  }