* [PATCH] bridge: don't try to update timers in case of broken MLD queries
@ 2013-08-05 22:32 Linus Lüssing
2013-08-05 22:42 ` Stephen Hemminger
2013-08-05 22:44 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Linus Lüssing @ 2013-08-05 22:32 UTC (permalink / raw)
To: bridge
Cc: Paul Bolle, Herbert Xu, netdev, linux-kernel, Adam Baker,
Stephen Hemminger, Linus Lüssing, David S. Miller,
Cong Wang
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@tiscali.nl>
Signed-off-by: Linus Lüssing <linus.luessing@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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] bridge: don't try to update timers in case of broken MLD queries
2013-08-05 22:32 [PATCH] bridge: don't try to update timers in case of broken MLD queries Linus Lüssing
@ 2013-08-05 22:42 ` Stephen Hemminger
2013-08-06 0:32 ` Linus Lüssing
2013-08-05 22:44 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2013-08-05 22:42 UTC (permalink / raw)
To: Linus Lüssing
Cc: Paul Bolle, Cong Wang, netdev, bridge, linux-kernel, Adam Baker,
David S. Miller, Herbert Xu
On Tue, 6 Aug 2013 00:32:05 +0200
Linus Lüssing <linus.luessing@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@tiscali.nl>
> Signed-off-by: Linus Lüssing <linus.luessing@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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] bridge: don't try to update timers in case of broken MLD queries
2013-08-05 22:42 ` Stephen Hemminger
@ 2013-08-06 0:32 ` Linus Lüssing
0 siblings, 0 replies; 4+ messages in thread
From: Linus Lüssing @ 2013-08-06 0:32 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Paul Bolle, Cong Wang, netdev, bridge, linux-kernel, Adam Baker,
linus.luessing, David S. Miller, Herbert Xu
On Mon, Aug 05, 2013 at 03:42:22PM -0700, Stephen Hemminger wrote:
> On Tue, 6 Aug 2013 00:32:05 +0200
> Linus Lüssing <linus.luessing@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@tiscali.nl>
> > Signed-off-by: Linus Lüssing <linus.luessing@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.
Because it isn't really necessary, it is basically included
in the pskb_may_pull() already, just like it is in the according IGMP
code path.
And I thought it'd be nicer to handle it the same way as in the
IGMP code path to avoid diverging too much.
Cheers, Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bridge: don't try to update timers in case of broken MLD queries
2013-08-05 22:32 [PATCH] bridge: don't try to update timers in case of broken MLD queries Linus Lüssing
2013-08-05 22:42 ` Stephen Hemminger
@ 2013-08-05 22:44 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2013-08-05 22:44 UTC (permalink / raw)
To: linus.luessing
Cc: pebolle, amwang, netdev, bridge, linux-kernel, linux, stephen, herbert
From: Linus Lüssing <linus.luessing@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@tiscali.nl>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-06 0:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-05 22:32 [PATCH] bridge: don't try to update timers in case of broken MLD queries Linus Lüssing
2013-08-05 22:42 ` Stephen Hemminger
2013-08-06 0:32 ` Linus Lüssing
2013-08-05 22:44 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).