lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Nov 2012 20:52:00 +0100 From: Lennert Buytenhek <buytenh@...tstofly.org> To: Stephen Hemminger <shemminger@...tta.com>, netdev@...r.kernel.org Cc: Steven Kath <steven.kath@...tta.com>, Anatoly Kaplan <anatoly.kaplan@...tta.com>, Arthur Xiong <arthur.xiong@...tta.com>, Chris Healy <cphealy@...il.com> Subject: stp issue and "bridge: send proper message_age in config BPDU" Hi! FWIW, I've been debugging an STP issue on an old product kernel tree that I couldn't find an upstream fix for, but after having debugged the issue, there does actually appear to be an upstream commit that makes the issue go away, but the commit message on that commit is somewhat unclear about what the issue is that it's fixing and why the given fix fixes it, and given that I spent considerable time debugging it I figured I'd send this out for the sake of the next person googling for this. The symptoms are pretty much what's described in this bug: https://bugzilla.vyatta.com/show_bug.cgi?id=7164 And the upstream commit is: https://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=0c03150e7ea8f7fcd03cfef29385e0010b22ee92 commit 0c03150e7ea8f7fcd03cfef29385e0010b22ee92 Author: stephen hemminger <shemminger@...tta.com> Date: Fri Jul 22 07:47:06 2011 +0000 bridge: send proper message_age in config BPDU What I was seeing was that as a non-root bridge, Linux STP would often fail to transmit BPDUs out of designated ports upon reception of a BPDU from an upstream port. br_received_config_bpdu() handles the received BPDU, and calls into br_record_config_information(), which resets the message age timer on this port to jiffies + (p->br->max_age - bpdu->message_age); When br_received_config_bpdu() then calls br_config_bpdu_generation(), the latter will call into br_transmit_config() for each enabled designated port, which will send out BPDUs with age br->max_age - (root->message_age_timer.expires - jiffies) + MESSAGE_AGE_INCR if we're not the root bridge, which if you plug in the previously computed timeout simplifies to bpdu->message_age + MESSAGE_AGE_INCR, which is exactly what we want it to be and this computation isn't wrong per se. The problem with the above logic, though, is that it fails to consider that mod_timer() can round up the timeout you give it (i.e. add timer slack), and that reading back root->message_age_timer.expires in br_transmit_config() won't necessarily return the value that was plugged into mod_timer() for this timer in br_record_config_information(). E.g. if mod_timer() decides to add 5 jiffies to the timeout, the message age value that br_transmit_config() will compute will be: br->max_age - (root->message_age_timer.expires - jiffies) + MESSAGE_AGE_INCR = br->max_age - (jiffies + (p->br->max_age - bpdu->message_age) + 5 - jiffies) + MESSAGE_AGE_INCR = br->max_age - (p->br->max_age - bpdu->message_age + 5) + MESSAGE_AGE_INCR = bpdu->message_age - 5 + MESSAGE_AGE_INCR Which will likely make the computed message age value negative. This message age is stored in a signed int, but is then compared against the bridge max age time: if (bpdu.message_age < br->max_age) { and br->max_age is an unsigned long, causing the comparison to be unsigned and always fail if the computed message age was negative, and no BPDU to be sent (causing our downstream neighbours to time us out after some time and etc). Commit 0c03150e7ea fixes the issue because it avoids reading back the expiration time (possibly with timer slack included) of a previously set timer. Disabling timer slack on the message age timer achieves the same thing (and is what I did initially): --- a/net/bridge/br_stp_timer.c +++ b/net/bridge/br_stp_timer.c @@ -158,6 +158,7 @@ void br_stp_port_timer_init(struct net_bridge_port *p) { setup_timer(&p->message_age_timer, br_message_age_timer_expired, (unsigned long) p); + set_timer_slack(&p->message_age_timer, 0); setup_timer(&p->forward_delay_timer, br_forward_delay_timer_expired, (unsigned long) p); thanks, Lennert -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists