On Dec 29, 10:09pm, al...@yandex.ru (Alexander Nasonov) wrote: -- Subject: Re: CVS commit: src/sys/net
| 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? I suppose by turning k unsigned, they really want to disable negative offsets. We could allow them if needed, but at that point it is better to make k signed. christos