Date: Fri, 2 Jul 1999 00:10:02 -0700 (PDT) From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/12484: [PATCH] bpf_filter() broken Message-ID: <199907020710.AAA48943@freefall.freebsd.org>
index | next in thread | raw e-mail
The following reply was made to PR kern/12484; it has been noted by GNATS.
From: Bruce Evans <bde@zeta.org.au>
To: eivind@freebsd.org, FreeBSD-gnats-submit@freebsd.org,
leres@ee.lbl.gov
Cc: mccanne@eecs.berkeley.edu, vern@ee.lbl.gov
Subject: Re: kern/12484: [PATCH] bpf_filter() broken
Date: Fri, 2 Jul 1999 17:00:13 +1000
> would not capture such packets. Debugging turned up a problem
> in this case:
>
> case BPF_LD|BPF_B|BPF_IND:
> k = X + pc->k;
> if (pc->k >= buflen || X >= buflen - k) {
>
> The conditional can be rewritten with "k" expanded:
>
> if (pc->k >= buflen || X >= buflen - (X + pc->k)) {
>
> which is the same as:
>
> if (pc->k >= buflen || X + X + pc->k >= buflen) {
>
> and is clearly wrong.
Oops. This should have been:
if (pc->k >= buflen || X >= buflen - p->k) {
which is the same modulo possible truncation mod 2^32 as each of the
following:
if (pc->k >= buflen || X + p->k >= buflen) {
if (pc->k >= buflen || k >= buflen) {
if (k >= buflen) {
The latter is the conditinal in rev.1.11.
> Other cases where "k" is initialized to "X + pc->k" appear to
> either be broken or at least contain unnecessary checks.
The other cases seem to be correct. The extra checks relative to 1.11
are to detect truncation for weird values of X and/or pc->k, e.g., X =
4, pc->k = (u_int32_t)-5 gives X + pc->k = 3 if u_int32_t is u_int.
I don't know if this much overflow checking is justified.
> The appended context changes are against 1.13 (it looks like
> 3.2-RELEASE shipped with 1.12).
>
>RCS file: RCS/bpf_filter.c,v
>retrieving revision 1.3
>diff -c -r1.3 bpf_filter.c
>*** /tmp/,RCSt1z29845 Thu Jul 1 19:43:31 1999
>- --- bpf_filter.c Thu Jul 1 19:42:40 1999
>***************
>*** 217,223 ****
>
> case BPF_LD|BPF_W|BPF_ABS:
> k = pc->k;
>! if (k > buflen || sizeof(int32_t) > buflen - k) {
> #ifdef KERNEL
> int merr;
>
>- --- 217,223 ----
>
> case BPF_LD|BPF_W|BPF_ABS:
> k = pc->k;
>! if (sizeof(int32_t) > buflen - k) {
> #ifdef KERNEL
> int merr;
>
If you reduce the amount of overflow checking back to 1.11 levels, then
change the checks back to the ones in 1.11. e.g.,
if (k + sizeof(int32_t) > buflen) {
The check:
if (sizeof(int32_t) > buflen - k) {
has fatal overflow problems if k > buflen and u_int32_t is u_int.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199907020710.AAA48943>