All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikolay Aleksandrov <razor@blackwall.org>
To: Baul Lee <baul.lee@xbow.com>
Cc: netdev@vger.kernel.org, bridge@lists.linux.dev,
	linux-kernel@vger.kernel.org, Ido Schimmel <idosch@nvidia.com>,
	Andrew Lunn <andrew@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH net v2] net: bridge: mrp: fix uninitialised bytes on the wire
Date: Thu, 30 Jul 2026 18:27:41 +0300	[thread overview]
Message-ID: <amttbcdZkuWVSdLE@penguin> (raw)
In-Reply-To: <20260729131941.10254-1-baul.lee@xbow.com>

On Wed, Jul 29, 2026 at 10:19:41PM +0900, Baul Lee wrote:
> br_mrp_alloc_test_skb() builds MRP test frames on an skb from
> dev_alloc_skb(), which does not clear 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;
> 
> so sub_tlv->length is never written, and the two trailing alignment bytes
> are appended with a bare skb_put() that does not clear them either.  The
> neighbouring oui and sub_opt regions are explicitly zeroed, so three
> uninitialised bytes are left in every MRA MRP_Test frame that goes out.
> 
> Put the sub-option TLV header and the alignment padding in a single
> skb_put_zero(), which clears both.  The AUTO_MGR sub-TLV carries no
> payload, so the zeroed length field is already the value it should have.
> 
> Fixes: f7458934b079 ("net: bridge: mrp: Update the Test frames for MRA")
> Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Baul Lee <baul.lee@xbow.com>
> ---
> v2:
>  - fold the alignment padding into a single skb_put_zero() for the
>    sub-option TLV header, dropping the explicit length assignment and the
>    second skb_put() (Nikolay Aleksandrov)
>  - drop the paragraph about observing the leak, and the Reported-by tags
> 
> Link to v1: https://lore.kernel.org/netdev/20260726062518.43774-1-baul.lee@xbow.com/
> 
>  net/bridge/br_mrp.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index 179d2470b..ef16d0703 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -224,11 +224,9 @@ static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
>  		sub_opt = skb_put(skb, sizeof(*sub_opt));
>  		memset(sub_opt, 0x0, sizeof(*sub_opt));
>  
> -		sub_tlv = skb_put(skb, sizeof(*sub_tlv));
> -		sub_tlv->type = BR_MRP_SUB_TLV_HEADER_TEST_AUTO_MGR;
> -
>  		/* 32 bit alligment shall be ensured therefore add 2 bytes */
> -		skb_put(skb, MRP_OPT_PADDING);
> +		sub_tlv = skb_put_zero(skb, sizeof(*sub_tlv) + MRP_OPT_PADDING);
> +		sub_tlv->type = BR_MRP_SUB_TLV_HEADER_TEST_AUTO_MGR;
>  	}
>  
>  	br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> -- 
> 2.53.0
> 

You should not send patches as replies, they should be sent on
their own. Aside from that the patch looks good to me:

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

      reply	other threads:[~2026-07-30 15:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-26  6:25 [PATCH net] net: bridge: mrp: fix uninitialised bytes on the wire Baul Lee
2026-07-26  9:54 ` Nikolay Aleksandrov
2026-07-26 17:00   ` Andrew Lunn
2026-07-29 13:19 ` [PATCH net v2] " Baul Lee
2026-07-30 15:27   ` Nikolay Aleksandrov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amttbcdZkuWVSdLE@penguin \
    --to=razor@blackwall.org \
    --cc=andrew@lunn.ch \
    --cc=baul.lee@xbow.com \
    --cc=bridge@lists.linux.dev \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.