Re: CVS commit: src/sys/net

[Alexander Nasonov <alnsn%yandex.ru@localhost>]

Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date:         Thu Dec 29 20:50:06 UTC 2011
> 
> Modified Files:
>       src/sys/net: bpf_filter.c
> 
> Log Message:
> PR/45751: Alexander Nasonov: No overflow check in BPF_LD|BPF_ABS
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.48 -r1.49 src/sys/net/bpf_filter.c
...  
> @@ -253,7 +254,8 @@ bpf_filter(const struct bpf_insn *pc, co
>  
>               case BPF_LD|BPF_H|BPF_IND:
>                       k = X + pc->k;
> -                     if (k + sizeof(int16_t) > buflen) {
> +                     if (pc->k > buflen || X > buflen - pc->k ||
> +                         sizeof(int16_t) > buflen - k) {
>  #ifdef _KERNEL
>                               int merr = 0;   /* XXX: GCC */

Not sure FreeBSD got BPF_IND case right. They basically disabled using
big positive values of pc->k for small negative values. They could just
copy code from BPF_ABS case:

> +                     if (k > buflen || sizeof(int16_t) > buflen - k) {

but they didn't. Can we assume that loads with negative offsets relative
to X (e.g. P[X-1:4]) are not allowed by bpf?

Alex