diff options
author | Eric Dumazet <eric.dumazet@gmail.com> | 2010-12-01 20:46:24 +0000 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2010-12-06 12:59:09 -0800 |
commit | 2d5311e4e8272fd398fc1cf278f12fd6dee4074b (patch) | |
tree | d86fcab7044baa6ec41ef87b8eca186ae6f9ea89 /net/core/filter.c | |
parent | ae9c416d686db74f67d73c1bebf1e3a7e8b3c5b5 (diff) | |
download | op-kernel-dev-2d5311e4e8272fd398fc1cf278f12fd6dee4074b.zip op-kernel-dev-2d5311e4e8272fd398fc1cf278f12fd6dee4074b.tar.gz |
filter: add a security check at install time
We added some security checks in commit 57fe93b374a6
(filter: make sure filters dont read uninitialized memory) to close a
potential leak of kernel information to user.
This added a potential extra cost at run time, while we can perform a
check of the filter itself, to make sure a malicious user doesnt try to
abuse us.
This patch adds a check_loads() function, whole unique purpose is to
make this check, allocating a temporary array of mask. We scan the
filter and propagate a bitmask information, telling us if a load M(K) is
allowed because a previous store M(K) is guaranteed. (So that
sk_run_filter() can possibly not read unitialized memory)
Note: this can uncover application bug, denying a filter attach,
previously allowed.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: Changli Gao <xiaosuo@gmail.com>
Acked-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core/filter.c')
-rw-r--r-- | net/core/filter.c | 72 |
1 files changed, 63 insertions, 9 deletions
diff --git a/net/core/filter.c b/net/core/filter.c index 054e286..ac4920a 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -166,11 +166,9 @@ unsigned int sk_run_filter(struct sk_buff *skb, const struct sock_filter *fentry u32 A = 0; /* Accumulator */ u32 X = 0; /* Index Register */ u32 mem[BPF_MEMWORDS]; /* Scratch Memory Store */ - unsigned long memvalid = 0; u32 tmp; int k; - BUILD_BUG_ON(BPF_MEMWORDS > BITS_PER_LONG); /* * Process array of filter instructions. */ @@ -318,12 +316,10 @@ load_b: X = K; continue; case BPF_S_LD_MEM: - A = (memvalid & (1UL << K)) ? - mem[K] : 0; + A = mem[K]; continue; case BPF_S_LDX_MEM: - X = (memvalid & (1UL << K)) ? - mem[K] : 0; + X = mem[K]; continue; case BPF_S_MISC_TAX: X = A; @@ -336,11 +332,9 @@ load_b: case BPF_S_RET_A: return A; case BPF_S_ST: - memvalid |= 1UL << K; mem[K] = A; continue; case BPF_S_STX: - memvalid |= 1UL << K; mem[K] = X; continue; default: @@ -425,6 +419,66 @@ load_b: } EXPORT_SYMBOL(sk_run_filter); +/* + * Security : + * A BPF program is able to use 16 cells of memory to store intermediate + * values (check u32 mem[BPF_MEMWORDS] in sk_run_filter()) + * As we dont want to clear mem[] array for each packet going through + * sk_run_filter(), we check that filter loaded by user never try to read + * a cell if not previously written, and we check all branches to be sure + * a malicious user doesnt try to abuse us. + */ +static int check_load_and_stores(struct sock_filter *filter, int flen) +{ + u16 *masks, memvalid = 0; /* one bit per cell, 16 cells */ + int pc, ret = 0; + + BUILD_BUG_ON(BPF_MEMWORDS > 16); + masks = kmalloc(flen * sizeof(*masks), GFP_KERNEL); + if (!masks) + return -ENOMEM; + memset(masks, 0xff, flen * sizeof(*masks)); + + for (pc = 0; pc < flen; pc++) { + memvalid &= masks[pc]; + + switch (filter[pc].code) { + case BPF_S_ST: + case BPF_S_STX: + memvalid |= (1 << filter[pc].k); + break; + case BPF_S_LD_MEM: + case BPF_S_LDX_MEM: + if (!(memvalid & (1 << filter[pc].k))) { + ret = -EINVAL; + goto error; + } + break; + case BPF_S_JMP_JA: + /* a jump must set masks on target */ + masks[pc + 1 + filter[pc].k] &= memvalid; + memvalid = ~0; + break; + case BPF_S_JMP_JEQ_K: + case BPF_S_JMP_JEQ_X: + case BPF_S_JMP_JGE_K: + case BPF_S_JMP_JGE_X: + case BPF_S_JMP_JGT_K: + case BPF_S_JMP_JGT_X: + case BPF_S_JMP_JSET_X: + case BPF_S_JMP_JSET_K: + /* a jump must set masks on targets */ + masks[pc + 1 + filter[pc].jt] &= memvalid; + masks[pc + 1 + filter[pc].jf] &= memvalid; + memvalid = ~0; + break; + } + } +error: + kfree(masks); + return ret; +} + /** * sk_chk_filter - verify socket filter code * @filter: filter to verify @@ -553,7 +607,7 @@ int sk_chk_filter(struct sock_filter *filter, int flen) switch (filter[flen - 1].code) { case BPF_S_RET_K: case BPF_S_RET_A: - return 0; + return check_load_and_stores(filter, flen); } return -EINVAL; } |