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

Signed-off-by: Kris Katterjohn <kjak@users.sourceforge.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/core/filter.c b/net/core/filter.c
index 3a10e0bc90e8a51ae4542ee4258f60f0d4509c64..8964d344558889bdf6c62c2d4383a265a38a433c 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -13,6 +13,7 @@
  * 2 of the License, or (at your option) any later version.
  *
  * Andi Kleen - Fix a few bad bugs and races.
+ * Kris Katterjohn - Added many additional checks in sk_chk_filter()
  */
 
 #include <linux/module.h>
@@ -250,7 +251,7 @@ load_b:
                        mem[fentry->k] = X;
                        continue;
                default:
-                       /* Invalid instruction counts as RET */
+                       WARN_ON(1);
                        return 0;
                }
 
@@ -283,8 +284,8 @@ load_b:
  *
  * Check the user's filter code. If we let some ugly
  * filter code slip through kaboom! The filter must contain
- * no references or jumps that are out of range, no illegal instructions
- * and no backward jumps. It must end with a RET instruction
+ * no references or jumps that are out of range, no illegal
+ * instructions, and must end with a RET instruction.
  *
  * Returns 0 if the rule set is legal or a negative errno code if not.
  */
@@ -300,38 +301,85 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
        for (pc = 0; pc < flen; pc++) {
                /* all jumps are forward as they are not signed */
                ftest = &filter[pc];
-               if (BPF_CLASS(ftest->code) == BPF_JMP) {
-                       /* but they mustn't jump off the end */
-                       if (BPF_OP(ftest->code) == BPF_JA) {
-                               /*
-                                * Note, the large ftest->k might cause loops.
-                                * Compare this with conditional jumps below,
-                                * where offsets are limited. --ANK (981016)
-                                */
-                               if (ftest->k >= (unsigned)(flen-pc-1))
-                                       return -EINVAL;
-                       } else {
-                               /* 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 */
-               if (ftest->code == (BPF_ALU|BPF_DIV|BPF_K) && ftest->k == 0)
-                       return -EINVAL;
+               /* Only allow valid instructions */
+               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 */
 
-               /* check that memory operations use valid addresses. */
-               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:   
+               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.
+                        * Compare this with conditional jumps below,
+                        * where offsets are limited. --ANK (981016)
+                        */
+                       if (ftest->k >= (unsigned)(flen-pc-1))
+                               return -EINVAL;
+                       break;
+
+               case BPF_JMP|BPF_JEQ|BPF_K:
+               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;
+                       break;
+
+               default:
+                       return -EINVAL;
                }
        }