Ulrich Spörlein <u...@freebsd.org> wrote in <20130724130046.gd9...@acme.spoerlein.net>:
uq> On Sat, 2013-07-20 at 16:46:51 +0000, Hiroki Sato wrote: uq> > Author: hrs uq> > Date: Sat Jul 20 16:46:51 2013 uq> > New Revision: 253504 uq> > URL: http://svnweb.freebsd.org/changeset/base/253504 uq> > uq> > Log: uq> > - Simplify getaddr() and print_getmsg() by using RTAX_* instead of RTA_* uq> > as the argument. uq> > - Reduce unnecessary loop in print_getmsg(). uq> > uq> > Modified: uq> > head/sbin/route/route.c uq> > uq> > Modified: head/sbin/route/route.c uq> > ============================================================================== uq> > --- head/sbin/route/route.c Sat Jul 20 15:58:43 2013 (r253503) uq> > +++ head/sbin/route/route.c Sat Jul 20 16:46:51 2013 (r253504) uq> > @@ -1105,7 +1105,7 @@ inet6_makenetandmask(struct sockaddr_in6 uq> > * returning 1 if a host address, 0 if a network address. uq> > */ uq> > static int uq> > -getaddr(int which, char *str, struct hostent **hpp, int nrflags) uq> > +getaddr(int idx, char *str, struct hostent **hpp, int nrflags) uq> > { uq> > struct sockaddr *sa; uq> > #if defined(INET) uq> > @@ -1130,36 +1130,16 @@ getaddr(int which, char *str, struct hos uq> > aflen = sizeof(struct sockaddr_dl); uq> > #endif uq> > } uq> > - rtm_addrs |= which; uq> > + rtm_addrs |= (1 << idx); uq> > uq> > - switch (which) { uq> > - case RTA_DST: uq> > - sa = (struct sockaddr *)&so[RTAX_DST]; uq> > - break; uq> > - case RTA_GATEWAY: uq> > - sa = (struct sockaddr *)&so[RTAX_GATEWAY]; uq> > - break; uq> > - case RTA_NETMASK: uq> > - sa = (struct sockaddr *)&so[RTAX_NETMASK]; uq> > - break; uq> > - case RTA_GENMASK: uq> > - sa = (struct sockaddr *)&so[RTAX_GENMASK]; uq> > - break; uq> > - case RTA_IFA: uq> > - sa = (struct sockaddr *)&so[RTAX_IFA]; uq> > - break; uq> > - case RTA_IFP: uq> > - sa = (struct sockaddr *)&so[RTAX_IFP]; uq> > - break; uq> > - default: uq> > + if (idx > RTAX_MAX) uq> > usage("internal error"); uq> > - /*NOTREACHED*/ uq> > - } uq> > + sa = (struct sockaddr *)&so[idx]; uq> uq> Coverity Scan flags this as an out-of-bounds write. RTAX_MAX is 8, so uq> idx can be up to 8 (inclusive) in the check above. Do you want to check uq> for idx >= RTAX_MAX maybe? idx is also signed ... uq> uq> Coverity CID is 1054779, btw. Sorry for the delay. Thank you for pointing out it. Yes, the check was wrong by one. Fixed in r253852. -- Hiroki
pgp_sN0Y0jTtV.pgp
Description: PGP signature