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

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

[Baul Lee]

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.

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>
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));
 		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)

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

[Nikolay Aleksandrov]

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)
> 

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

[Andrew Lunn]

On Sun, Jul 26, 2026 at 12:54:33PM +0300, Nikolay Aleksandrov wrote:
> 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).

Hi Baul

Please be careful with this. AI slop will quickly damage your
reputation with Reviewers and Maintainers. If you have a poor
reputation, your patches are likely to be slower to get reviewed, get
more examination and more minor problems pointed out. AI is a useful
tool, but it is just a tool. You personally are responsible for
everything in the patch, so it is up to you to decided if what the AI
has generated is correct/useful.

> > 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.

Please handle this for all the other patches you have posted to
netdev.

It is a good idea to only post one patch, get review comments for all
the errors you make, and learn from them. Only once you are confident
you are likely to get everything correct can you start posting
multiple patches in parallel. Posting lots of patches with the same
error will also quickly damage your reputation.

	Andrew