[PATCH] net: bridge: mcast: read ngrec once in igmp3/mld2 report
From: Joy Gu <hidden>
Date: 2022-12-20 02:49:03
Also in:
bridge, lkml
Subsystem:
ethernet bridge, networking [general], the rest · Maintainers:
Nikolay Aleksandrov, Ido Schimmel, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
In br_ip4_multicast_igmp3_report() and br_ip6_multicast_mld2_report(), "ih" or "mld2r" is a pointer into the skb header. It's dereferenced to get "num", which is used in the for-loop condition that follows. Compilers are free to not spend a register on "num" and dereference that pointer every time "num" would be used, i.e. every loop iteration. Which would be a bug if pskb_may_pull() (called by ip_mc_may_pull() or ipv6_mc_may_pull() in the loop body) were to change pointers pointing into the skb header, e.g. by freeing "skb->head". We can avoid this by using READ_ONCE(). Suggested-by: Joern Engel <redacted> Signed-off-by: Joy Gu <redacted> --- net/bridge/br_multicast.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 48170bd3785e..2ac4b099e00d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c@@ -2624,11 +2624,11 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge_mcast *brmctx, bool changed = false; int err = 0; u16 nsrcs; ih = igmpv3_report_hdr(skb); - num = ntohs(ih->ngrec); + num = ntohs(READ_ONCE(ih->ngrec)); len = skb_transport_offset(skb) + sizeof(*ih); for (i = 0; i < num; i++) { len += sizeof(*grec); if (!ip_mc_may_pull(skb, len))
@@ -2750,11 +2750,11 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx, if (!ipv6_mc_may_pull(skb, sizeof(*mld2r))) return -EINVAL; mld2r = (struct mld2_report *)icmp6_hdr(skb); - num = ntohs(mld2r->mld2r_ngrec); + num = ntohs(READ_ONCE(mld2r->mld2r_ngrec)); len = skb_transport_offset(skb) + sizeof(*mld2r); for (i = 0; i < num; i++) { __be16 *_nsrcs, __nsrcs; u16 nsrcs;
--
2.39.0