Date: Thu, 13 Sep 2001 04:50:02 -0700 (PDT) From: Dima Dorfman <dima@unixfreak.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/30524: 4.4-RC4: panic in icmp_reflect [WITH PATCH] Message-ID: <200109131150.f8DBo2C56500@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/30524; it has been noted by GNATS.
From: Dima Dorfman <dima@unixfreak.org>
To: Lars Eggert <larse@isi.edu>
Cc: freebsd-gnats-submit@FreeBSD.org, wollman@freebsd.org
Subject: Re: kern/30524: 4.4-RC4: panic in icmp_reflect [WITH PATCH]
Date: Thu, 13 Sep 2001 04:49:13 -0700
Lars Eggert <larse@isi.edu> wrote:
> --- plain Wed Sep 12 09:47:58 2001
> +++ ip_icmp.c Wed Sep 12 09:37:54 2001
> @@ -639,9 +639,26 @@
> /*
> * The following happens if the packet was not addressed to us,
> * and was received on an interface with no IP address.
> + *
> + * Prior versions simply set ia = in_ifaddrhead.tqh_first if it
> + * was zero here. When in_ifaddrhead.tqh_first is also zero
> + * (pointer gets dereferenced below), the kernel panics. It looks
> + * like this can happen with PC-card interfaces, but I have not
> + * investigated this fully.
> + *
> + * Instead, iterate over the TAILQ to find the first non-zero
> + * interface address, and use that. If none can be found, skip
> + * sending the ICMP packet.
> + * larse@isi.edu
> */
> - if (ia == (struct in_ifaddr *)0)
> - ia = in_ifaddrhead.tqh_first;
> + if (ia == (struct in_ifaddr *)0) {
> + TAILQ_FOREACH(ia, &in_ifaddrhead, ia_link)
> + if (ia != (struct in_ifaddr *)0)
> + break;
This check doesn't make sense, because TAILQ_FOREACH expands
(effectively) into:
for (ia = in_ifaddrhead.tqh_first; ia != NULL; ia = ia->tqh_next)
/* I might've gotten some variable names wrong here. */
Thus, the if() inside it will *always* fail. Essentially what you
want to do is check if the TAILQ is empty, and fail otherwise. But...
Right now, the code in question assumes that if a packet gets to it,
there must be a valid interface. However, this assumption was broken
in rev. 1.114 of ip_input.c, which says:
----------------------------
revision 1.114
date: 1999/02/09 16:55:46; author: wollman; state: Exp; lines: +8 -10
After wading in the cesspool of ip_input for an hour, I have managed to
convince myself that nothing will break if we permit IP input while
interface addresses are unconfigured. (At worst, they will hit some
ULP's PCB scan and fail if nobody is listening.) So, remove the restriction
that addresses must be configured before packets can be input. Assume
that any unicast packet we receive while unconfigured is potentially ours.
----------------------------
Unforunately, the commit log doesn't provide any rationale for the
change, just reassurance that it probably doesn't break anything.
Garrett (cc'd), can you provide any insight on this? Was your
intention to filter down TAILQ_EMPTY(&in_ifaddrhead) checks below
in_input, and, if so, why? If the only recourse in this case is to
bail out, why shouldn't it be done in in_input? Or should this code
try to do something else?
> + if (ia == (struct in_ifaddr *)0)
> + /* did not find any valid interface address */
You would also need to free `m' here.
> + goto done;
> + }
> t = IA_SIN(ia)->sin_addr;
> ip->ip_src = t;
> ip->ip_ttl = ip_defttl;
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200109131150.f8DBo2C56500>