Linus Lüssing
2013-Aug-05 22:32 UTC
[Bridge] [PATCH] bridge: don't try to update timers in case of broken MLD queries
Currently we are reading an uninitialized value for the max_delay
variable when snooping an MLD query message of invalid length and would
update our timers with that.
Fixing this by simply ignoring such broken MLD queries (just like we do
for IGMP already).
This is a regression introduced by:
"bridge: disable snooping if there is no querier" (b00589af3b04)
Reported-by: Paul Bolle <pebolle at tiscali.nl>
Signed-off-by: Linus L?ssing <linus.luessing at web.de>
---
net/bridge/br_multicast.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 61c5e81..08e576a 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1195,7 +1195,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
if (max_delay)
group = &mld->mld_mca;
- } else if (skb->len >= sizeof(*mld2q)) {
+ } else {
if (!pskb_may_pull(skb, sizeof(*mld2q))) {
err = -EINVAL;
goto out;
--
1.7.10.4
Stephen Hemminger
2013-Aug-05 22:42 UTC
[Bridge] [PATCH] bridge: don't try to update timers in case of broken MLD queries
On Tue, 6 Aug 2013 00:32:05 +0200 Linus L?ssing <linus.luessing at web.de> wrote:> Currently we are reading an uninitialized value for the max_delay > variable when snooping an MLD query message of invalid length and would > update our timers with that. > > Fixing this by simply ignoring such broken MLD queries (just like we do > for IGMP already). > > This is a regression introduced by: > "bridge: disable snooping if there is no querier" (b00589af3b04) > > Reported-by: Paul Bolle <pebolle at tiscali.nl> > Signed-off-by: Linus L?ssing <linus.luessing at web.de> > --- > net/bridge/br_multicast.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 61c5e81..08e576a 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1195,7 +1195,7 @@ static int br_ip6_multicast_query(struct net_bridge *br, > max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay)); > if (max_delay) > group = &mld->mld_mca; > - } else if (skb->len >= sizeof(*mld2q)) { > + } else { > if (!pskb_may_pull(skb, sizeof(*mld2q))) { > err = -EINVAL; > goto out;Why not use else if here, other than that looks great.
David Miller
2013-Aug-05 22:44 UTC
[Bridge] [PATCH] bridge: don't try to update timers in case of broken MLD queries
From: Linus L?ssing <linus.luessing at web.de> Date: Tue, 6 Aug 2013 00:32:05 +0200> Currently we are reading an uninitialized value for the max_delay > variable when snooping an MLD query message of invalid length and would > update our timers with that. > > Fixing this by simply ignoring such broken MLD queries (just like we do > for IGMP already). > > This is a regression introduced by: > "bridge: disable snooping if there is no querier" (b00589af3b04) > > Reported-by: Paul Bolle <pebolle at tiscali.nl> > Signed-off-by: Linus L?ssing <linus.luessing at web.de>Applied, thanks.