Bug 17939
| Summary: | routed calls ntohs twice on the same field | ||
|---|---|---|---|
| Product: | Base System | Reporter: | msmith <msmith> |
| Component: | bin | Assignee: | Sheldon Hearn <sheldonh> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | Unspecified | ||
| Hardware: | Any | ||
| OS: | Any | ||
Responsible Changed From-To: freebsd-bugs->sheldonh I've mailed the author (Vernon Schryver <vjs@calcite.rhyolite.com>) and am awaiting his feedback. Feedback from the author... ------- Forwarded Message Date: Sat, 15 Apr 2000 14:09:19 -0600 (MDT) From: Vernon Schryver <vjs@calcite.rhyolite.com> Message-Id: <200004152009.OAA27229@calcite.rhyolite.com> To: sheldonh@uunet.co.za Subject: Re: byte-swapping problem in FreeBSD routed > From: Sheldon Hearn <sheldonh@uunet.co.za> > To: "Vernon J. Schryver" <vjs@mica.denver.sgi.com> > Date: Wed, 12 Apr 2000 11:33:30 +0200 > Hi Vernon, > > We've received a problem report (PR) which proposes that the lifetime > member of an advertisement is byte-swapped twice. Could you take a look > at the PR and comment? > > http://www.freebsd.org/cgi-bin/query-pr.cgi?pr=17939 Yes, it's a bug. I think it's more serious than that PR suggests. I've built a bundle of version 2.20 in ftp://ftp.rhyolite.com/src/routed.tar.Z Besides your fix, I've cleaned up a few warnings from a new version of gcc. I also added some comments about the byte order of the lifetime. I wish I knew of a way to resolve the warning from the va_start() macro casting a const in typical varargs functions that use printf-style patterns. vjs ------- End of Forwarded Message State Changed From-To: open->analyzed Vernon's routed-2.21 fixes this problem and has been imported on the VJS vendor branch for HEAD. In a few weeks, I plan to merge it onto the RELENG_4 branch. State Changed From-To: analyzed->closed This problem was fixed on the RELENG_4 branch on 2000/08/14! |
In sbin/routed/rdisc.c, parse_ad() is called as follows: parse_ad(from.sin_addr.s_addr, wp[0], wp[1], ntohs(p->ad.icmp_ad_life), ifp); Thus, when we are in parse_ad() the 4th arg (life) is already in host order. Down at the bottom of parse_ad() we have this: new_drp->dr_life = ntohs(life); It looks to me like this call to ntohs() ends up converting it back to network order. This causes routed to use the wrong value when it uses dr_life elsewhere. Fix: Don't call ntohs() in parse_ad(). How-To-Repeat: This was found by code inspection. The default value for life is 1800 which is 2055 when byte swapped so most people probably never noticed.