Skip site navigation (1)Skip section navigation (2)
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>