Date: Thu, 10 Feb 2000 13:18:37 -0800 (PST) From: Jin Guojun (FTG staff) <jin@iss-p2.lbl.gov> To: FreeBSD-gnats-submit@freebsd.org Subject: kern/16644: Bad comparsion expression in bpf_filter.c Message-ID: <200002102118.NAA00545@iss-p2.lbl.gov>
next in thread | raw e-mail | index | archive | help
>Number: 16644
>Category: kern
>Synopsis: Bad comparsion expression in bpf_filter.c
>Confidential: yes
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Feb 10 13:20:02 PST 2000
>Closed-Date:
>Last-Modified:
>Originator: Jin Guojun (FTG staff)
>Release: FreeBSD 4.0-CURRENT
>Organization:
>Environment:
FreeBSD 4.0-CURRENT for release
It acturally exists in 3.x, but I would not bother for 3.x, but
really want it to be fix before 4.0-RELEASE in 4.0.
>Description:
I seems to remember there was a discussion about this expression change
from the original one at a point, but since I had not time to look at it,
I always use my original to replace the FreeBSD one here.
Since the original one had a little trouble to compile under 4.0-CURRENT now,
JUST samll thing that KERNEL changed to _KERNEL, I decide to use the FreeBSD
modified one, but got completed failure. The reason will discuss here:
segment of code in /sys/net/bpf_filter.c: (I added two printf to show info.)
...
case BPF_LD|BPF_H|BPF_ABS:
k = pc->k;
printf("k = %d, buflen = %d, b-k %d : k>b %d, s > b-k %d : reg % d\n",
k, buflen, buflen-k, k > buflen,
sizeof(short) > buflen - k, k + sizeof(short) > buflen);
{ int K = k;
printf("K = %d, buflen = %d, b-K %d : K>b %d, s > b-K %d : reg % d\n",
K, buflen, buflen-K, K > buflen,
sizeof(short) > buflen - K, K + sizeof(short) > buflen) ;
}
/* real problem is HERE XXX */
if (k > buflen || sizeof(short) > buflen - k) {
...
} else {
...
}
[1] /kernel: k = 6, buflen = 40, b-k 34 : k>b 0, s > b-k 0 : reg 0
[2] /kernel: K = 6, buflen = 40, b-K 34 : K>b 0, s > b-K 0 : reg 0
[3]/kernel: k = -1, buflen = 40, b-k 41 : k>b 1, s > b-k 0 : reg 0
[4]/kernel: K = -1, buflen = 40, b-K 41 : K>b 1, s > b-K 0 : reg 0
Two problems here:
(1) this expression is not been compiled correctly somehow by compiler.
in output line [4]; K>b shoud be false 0, but not true 1.
(2) Critical part:
This expression broken the original design in two place.
<1> Since you defined all variables are u_int##, then you
mean every var is positive #, then above expression is
bogus.
Translate this expression to real #:
(A > 10 || 2 > 10 - A)
=== (A > 10 || A > 10 - 2)
=== (A > 10 || A > 8)
=== (A > 8)
Here is go back to the original expression
(A + 2 > 10)
=== (k + sizeof(short) > buflen)
<2> The negtive k or pc->k was intended to be used n BPF filter.
This seems too have changed in bpf.h from int to u_int.
Also, the (k > buflen || sizeof(short) > buflen - k) increasing
the complexity in brach statement which is very CPU cost.
BPF is designed for light weight process.
This incorrect expression has replaced every original expression through
the bpf_filter.c. It need to alter back.
>How-To-Repeat:
>Fix:
If whoever make expression change agrees my explaination, please
reply this PR before the formal release. Make sure we have time
to change it back. Then I will send the "diff -c" for the patch.
>Release-Note:
>Audit-Trail:
>Unformatted:
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?200002102118.NAA00545>