From owner-freebsd-bugs Thu Sep 13 4:50: 9 2001 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 4DBB037B40E for ; Thu, 13 Sep 2001 04:50:02 -0700 (PDT) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.4/8.11.4) id f8DBo2C56500; Thu, 13 Sep 2001 04:50:02 -0700 (PDT) (envelope-from gnats) Date: Thu, 13 Sep 2001 04:50:02 -0700 (PDT) Message-Id: <200109131150.f8DBo2C56500@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Dima Dorfman Subject: Re: kern/30524: 4.4-RC4: panic in icmp_reflect [WITH PATCH] Reply-To: Dima Dorfman Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org The following reply was made to PR kern/30524; it has been noted by GNATS. From: Dima Dorfman To: Lars Eggert 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 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