Commit 93699863 authored by Kris Katterjohn's avatar Kris Katterjohn Committed by David S. Miller

[NET]: More instruction checks fornet/core/filter.c

Signed-off-by: default avatarKris Katterjohn <kjak@users.sourceforge.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 181a46a5
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
* 2 of the License, or (at your option) any later version. * 2 of the License, or (at your option) any later version.
* *
* Andi Kleen - Fix a few bad bugs and races. * Andi Kleen - Fix a few bad bugs and races.
* Kris Katterjohn - Added many additional checks in sk_chk_filter()
*/ */
#include <linux/module.h> #include <linux/module.h>
...@@ -250,7 +251,7 @@ load_b: ...@@ -250,7 +251,7 @@ load_b:
mem[fentry->k] = X; mem[fentry->k] = X;
continue; continue;
default: default:
/* Invalid instruction counts as RET */ WARN_ON(1);
return 0; return 0;
} }
...@@ -283,8 +284,8 @@ load_b: ...@@ -283,8 +284,8 @@ load_b:
* *
* Check the user's filter code. If we let some ugly * Check the user's filter code. If we let some ugly
* filter code slip through kaboom! The filter must contain * filter code slip through kaboom! The filter must contain
* no references or jumps that are out of range, no illegal instructions * no references or jumps that are out of range, no illegal
* and no backward jumps. It must end with a RET instruction * instructions, and must end with a RET instruction.
* *
* Returns 0 if the rule set is legal or a negative errno code if not. * Returns 0 if the rule set is legal or a negative errno code if not.
*/ */
...@@ -300,9 +301,60 @@ int sk_chk_filter(struct sock_filter *filter, int flen) ...@@ -300,9 +301,60 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
for (pc = 0; pc < flen; pc++) { for (pc = 0; pc < flen; pc++) {
/* all jumps are forward as they are not signed */ /* all jumps are forward as they are not signed */
ftest = &filter[pc]; ftest = &filter[pc];
if (BPF_CLASS(ftest->code) == BPF_JMP) {
/* but they mustn't jump off the end */ /* Only allow valid instructions */
if (BPF_OP(ftest->code) == BPF_JA) { switch (ftest->code) {
case BPF_ALU|BPF_ADD|BPF_K:
case BPF_ALU|BPF_ADD|BPF_X:
case BPF_ALU|BPF_SUB|BPF_K:
case BPF_ALU|BPF_SUB|BPF_X:
case BPF_ALU|BPF_MUL|BPF_K:
case BPF_ALU|BPF_MUL|BPF_X:
case BPF_ALU|BPF_DIV|BPF_X:
case BPF_ALU|BPF_AND|BPF_K:
case BPF_ALU|BPF_AND|BPF_X:
case BPF_ALU|BPF_OR|BPF_K:
case BPF_ALU|BPF_OR|BPF_X:
case BPF_ALU|BPF_LSH|BPF_K:
case BPF_ALU|BPF_LSH|BPF_X:
case BPF_ALU|BPF_RSH|BPF_K:
case BPF_ALU|BPF_RSH|BPF_X:
case BPF_ALU|BPF_NEG:
case BPF_LD|BPF_W|BPF_ABS:
case BPF_LD|BPF_H|BPF_ABS:
case BPF_LD|BPF_B|BPF_ABS:
case BPF_LD|BPF_W|BPF_LEN:
case BPF_LD|BPF_W|BPF_IND:
case BPF_LD|BPF_H|BPF_IND:
case BPF_LD|BPF_B|BPF_IND:
case BPF_LD|BPF_IMM:
case BPF_LDX|BPF_W|BPF_LEN:
case BPF_LDX|BPF_B|BPF_MSH:
case BPF_LDX|BPF_IMM:
case BPF_MISC|BPF_TAX:
case BPF_MISC|BPF_TXA:
case BPF_RET|BPF_K:
case BPF_RET|BPF_A:
break;
/* Some instructions need special checks */
case BPF_ALU|BPF_DIV|BPF_K:
/* check for division by zero */
if (ftest->k == 0)
return -EINVAL;
break;
case BPF_LD|BPF_MEM:
case BPF_LDX|BPF_MEM:
case BPF_ST:
case BPF_STX:
/* check for invalid memory addresses */
if (ftest->k >= BPF_MEMWORDS)
return -EINVAL;
break;
case BPF_JMP|BPF_JA:
/* /*
* Note, the large ftest->k might cause loops. * Note, the large ftest->k might cause loops.
* Compare this with conditional jumps below, * Compare this with conditional jumps below,
...@@ -310,30 +362,26 @@ int sk_chk_filter(struct sock_filter *filter, int flen) ...@@ -310,30 +362,26 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
*/ */
if (ftest->k >= (unsigned)(flen-pc-1)) if (ftest->k >= (unsigned)(flen-pc-1))
return -EINVAL; return -EINVAL;
} else { break;
/* for conditionals both must be safe */
if (pc + ftest->jt +1 >= flen ||
pc + ftest->jf +1 >= flen)
return -EINVAL;
}
}
/* check for division by zero -Kris Katterjohn 2005-10-30 */ case BPF_JMP|BPF_JEQ|BPF_K:
if (ftest->code == (BPF_ALU|BPF_DIV|BPF_K) && ftest->k == 0) case BPF_JMP|BPF_JEQ|BPF_X:
case BPF_JMP|BPF_JGE|BPF_K:
case BPF_JMP|BPF_JGE|BPF_X:
case BPF_JMP|BPF_JGT|BPF_K:
case BPF_JMP|BPF_JGT|BPF_X:
case BPF_JMP|BPF_JSET|BPF_K:
case BPF_JMP|BPF_JSET|BPF_X:
/* for conditionals both must be safe */
if (pc + ftest->jt + 1 >= flen ||
pc + ftest->jf + 1 >= flen)
return -EINVAL; return -EINVAL;
break;
/* check that memory operations use valid addresses. */ default:
if (ftest->k >= BPF_MEMWORDS) {
/* but it might not be a memory operation... */
switch (ftest->code) {
case BPF_ST:
case BPF_STX:
case BPF_LD|BPF_MEM:
case BPF_LDX|BPF_MEM:
return -EINVAL; return -EINVAL;
} }
} }
}
/* /*
* The program must end with a return. We don't care where they * The program must end with a return. We don't care where they
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment