Date: Tue, 4 May 2004 06:35:00 -0700 From: Luigi Rizzo <rizzo@icir.org> To: Colin Percival <colin.percival@wadham.ox.ac.uk> Cc: freebsd-net@freebsd.org Subject: Re: [patch] Verify that ifaddr_byindex(foo) != NULL Message-ID: <20040504063500.A37862@xorpc.icir.org> In-Reply-To: <6.1.0.6.1.20040504133711.03d1ce18@popserver.sfu.ca>; from colin.percival@wadham.ox.ac.uk on Tue, May 04, 2004 at 01:42:20PM %2B0100 References: <6.1.0.6.1.20040504133711.03d1ce18@popserver.sfu.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, May 04, 2004 at 01:42:20PM +0100, Colin Percival wrote:
> Could someone confirm for me that this looks sensible? I don't
> know anything about this code, but if we're going to check that
> 0 < ifp->if_index <= if_index, it seems that we should also be
> checking that ifp->if_index corresponds to an interface which
> still exists (rather than a gap left behind when an interface was
> removed).
well, the problem here and elsewhere is whether we trust the rcvif
field or not -- if we do, we must assume that if_index and ifadd_byindex()
are all valid, because they are all set consistently in if_attach().
If the interface is gone, ifp is already bogus thus there is no
point to check.
So i'd vote to remove all the bogus checks here and elsewhere, rather
than add newer ones.
cheers
luigi
> Colin Percival
>
> Index: src/sys/netinet/ip_input.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.270
> diff -u -p -r1.270 ip_input.c
> --- src/sys/netinet/ip_input.c 2 May 2004 15:10:16 -0000 1.270
> +++ src/sys/netinet/ip_input.c 4 May 2004 12:37:02 -0000
> @@ -2053,7 +2053,8 @@ ip_savecontrol(inp, mp, ip, m)
> struct sockaddr_dl *sdl2 = &sdlbuf.sdl;
>
> if (((ifp = m->m_pkthdr.rcvif))
> - && ( ifp->if_index && (ifp->if_index <= if_index))) {
> + && ( ifp->if_index && (ifp->if_index <= if_index)) &&
> + (ifaddr_byindex(ifp->if_index) != NULL)) {
> sdp = (struct sockaddr_dl *)
> (ifaddr_byindex(ifp->if_index)->ifa_addr);
> /*
>
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040504063500.A37862>