From owner-freebsd-bugs  Sat Sep  1  2:10: 8 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 8B1FD37B406
	for <freebsd-bugs@hub.freebsd.org>; Sat,  1 Sep 2001 02:10:01 -0700 (PDT)
Received: (from gnats@localhost)
	by freefall.freebsd.org (8.11.4/8.11.4) id f819A1v53924;
	Sat, 1 Sep 2001 02:10:01 -0700 (PDT)
	(envelope-from gnats)
Date: Sat, 1 Sep 2001 02:10:01 -0700 (PDT)
Message-Id: <200109010910.f819A1v53924@freefall.freebsd.org>
To: freebsd-bugs@FreeBSD.org
Cc: 
From: Bruce Evans <bde@zeta.org.au>
Subject: Re: kern/16644: Bad comparsion expression in bpf_filter.c
Reply-To: Bruce Evans <bde@zeta.org.au>
Sender: owner-freebsd-bugs@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-bugs.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-bugs>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-bugs>
X-Loop: FreeBSD.org

The following reply was made to PR kern/16644; it has been noted by GNATS.

From: Bruce Evans <bde@zeta.org.au>
To: "Jin Guojun[ITG]" <j_guojun@lbl.gov>
Cc: <mike@FreeBSD.ORG>, <freebsd-gnats-submit@FreeBSD.ORG>
Subject: Re: kern/16644: Bad comparsion expression in bpf_filter.c
Date: Sat, 1 Sep 2001 19:01:12 +1000 (EST)

 On Fri, 31 Aug 2001, Jin Guojun[ITG] wrote:
 
 > It is still there. I have replied this to the discussion and
 > got no response. For example, in line 220, ">" line is equal to
 > if (k > buflen || k + sizeof(int32_t) > buflen) {
 > or
 > if (k > buflen || k > buflen - sizeof(int32_t)) {
 >
 > if K > BUFLEN then K must > BUFLEN - 4
 
 This doesn't follow.  Example: K = 4U, BUFLEN = 2U, BUFLEN - 4 = UINT_MAX - 2.
 BUFLEN - 4 overflows.  I'm not sure if this overflow can give undefined
 behaviour instead of UINT_MAX - 2.
 
 > so we only want to judge if (k > buflen - sizeof(int32_t)) {
 > which is the "<" of line 220 -- if (k + sizeof(int32_t) > buflen) {
 >
 > Right? rests are ditto. The original design is correct.
 
 No.  Examples:
 
 (1) k = UINT_MAX - 2, buflen = 8U.  Then k + sizeof(int32_t) = 2U, which
     is <= buflen.  But k is much larger than buflen - sizeof(int32_t).
     Here arithmetic overflow occurs in the buffer overflow check and
     the buffer overflow check gives the wrong result.  The overflow
     checks were rewritten in FreeBSD to avoid arithmetic overflow.
 (2) k = -2 (signed int, as in the original design), buflen = 8U.  Then
     k + sizeof(int32_t) = 2U (no change).  This is the same overflow as
     in (1).  k first gets promoted to UINT_MAX - 2, then the addition
     overflows.
 
 > The real problem is at line 550. K is outside 0-BPF_MEMWORDS, not just >.
 > ...
 > 547c550
 > <                   (p->k >= BPF_MEMWORDS || p->k < 0))
 > ---
 > >                   p->k >= BPF_MEMWORDS)
 
 This patch is reversed (it gives the FreeBSD change).  Here FreeBSD
 simplified the check instead of complicating it.  p->k < 0 can't happen
 because p->k is unsigned.  The compiler should optimize away the
 complication and generate identical code, but it migh warn about a
 bogus comparison of an unsigned value with 0.
 
 Bruce
 

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message