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 <[email protected]> Signed-off-by: Joy Gu <[email protected]> --- 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
