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 <joern@xxxxxxxxxxxxxxx>
Signed-off-by: Joy Gu <jgu@xxxxxxxxxxxxxxx>
---
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
