Re: [PATCH net] net: bridge: mrp: fix uninitialised bytes on the wire

[Nikolay Aleksandrov <razor@blackwall.org>]

On Sun, Jul 26, 2026 at 03:25:18PM +0900, Baul Lee wrote:
> br_mrp_alloc_test_skb() builds MRP test frames on an skb from
> dev_alloc_skb(), which does not zero the linear data area.  On the MRA
> ring-role branch the sub-option TLV header is appended with
> 
> 	sub_tlv = skb_put(skb, sizeof(*sub_tlv));
> 	sub_tlv->type = BR_MRP_SUB_TLV_HEADER_TEST_AUTO_MGR;
> 
> leaving sub_tlv->length unwritten, and the two trailing alignment bytes
> are appended with a bare skb_put() that neither writes nor clears them.
> The surrounding oui and sub_opt regions are explicitly memset(0), which
> bounds the exposure to exactly these three bytes.
> 
> Every MRA MRP_Test frame therefore carries three bytes of stale
> page-allocator memory, at frame offsets 65 to 67, to any observer of the
> MRP control traffic.  A capture on a kernel without
> CONFIG_INIT_ON_ALLOC_DEFAULT_ON shows those bytes varying frame to frame
> and, after a page-allocator spray, carrying the sprayed pattern; the
> same reproducer on an otherwise identical CONFIG_INIT_ON_ALLOC_DEFAULT_ON
> kernel leaks nothing, confirming the source is uninitialised allocation
> memory.  Reaching it needs CAP_NET_ADMIN, which is self-satisfiable on a
> stock kernel through unprivileged user and network namespaces.

Drop this entire unnecessary paragraph (slop).

> 
> Assign the sub-option TLV length explicitly, which is 0 as the AUTO_MGR
> sub-TLV carries no payload, and append the alignment padding with
> skb_put_zero().
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: f7458934b079 ("net: bridge: mrp: Update the Test frames for MRA")
> Reported-by: Federico Kirschbaum <federico.kirschbaum@xbow.com>
> Reported-by: Baul Lee <baul.lee@xbow.com>

You don't need a reported-by tag since you've already signed off
the patch.

> Cc: stable@vger.kernel.org
> Signed-off-by: Baul Lee <baul.lee@xbow.com>
> ---
>  net/bridge/br_mrp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index 3f7126a7d720..a5548f475604 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -226,9 +226,10 @@ static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
>  
>  		sub_tlv = skb_put(skb, sizeof(*sub_tlv));

if you use skb_put_zero here, you can drop the explicit zeroing below,
in fact you can add the MRP_OPT_PADDING as well and move the comment
above it then drop the second skb_put_zero entirely

>  		sub_tlv->type = BR_MRP_SUB_TLV_HEADER_TEST_AUTO_MGR;
> +		sub_tlv->length = 0x0;
>  
>  		/* 32 bit alligment shall be ensured therefore add 2 bytes */
> -		skb_put(skb, MRP_OPT_PADDING);
> +		skb_put_zero(skb, MRP_OPT_PADDING);
>  	}
>  
>  	br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> -- 
> 2.50.1 (Apple Git-155)
>