lkml.org 
[lkml]   [2010]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[270/289] filter: make sure filters dont read uninitialized memory
    2.6.36-stable review patch.  If anyone has any objections, please let us know.

    ------------------

    From: David S. Miller <davem@davemloft.net>

    commit 57fe93b374a6b8711995c2d466c502af9f3a08bb upstream.

    There is a possibility malicious users can get limited information about
    uninitialized stack mem array. Even if sk_run_filter() result is bound
    to packet length (0 .. 65535), we could imagine this can be used by
    hostile user.

    Initializing mem[] array, like Dan Rosenberg suggested in his patch is
    expensive since most filters dont even use this array.

    Its hard to make the filter validation in sk_chk_filter(), because of
    the jumps. This might be done later.

    In this patch, I use a bitmap (a single long var) so that only filters
    using mem[] loads/stores pay the price of added security checks.

    For other filters, additional cost is a single instruction.

    [ Since we access fentry->k a lot now, cache it in a local variable
    and mark filter entry pointer as const. -DaveM ]

    Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
    Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    net/core/filter.c | 64 +++++++++++++++++++++++++++++-------------------------
    1 file changed, 35 insertions(+), 29 deletions(-)

    --- a/net/core/filter.c
    +++ b/net/core/filter.c
    @@ -112,39 +112,41 @@ EXPORT_SYMBOL(sk_filter);
    */
    unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int flen)
    {
    - struct sock_filter *fentry; /* We walk down these */
    void *ptr;
    u32 A = 0; /* Accumulator */
    u32 X = 0; /* Index Register */
    u32 mem[BPF_MEMWORDS]; /* Scratch Memory Store */
    + unsigned long memvalid = 0;
    u32 tmp;
    int k;
    int pc;

    + BUILD_BUG_ON(BPF_MEMWORDS > BITS_PER_LONG);
    /*
    * Process array of filter instructions.
    */
    for (pc = 0; pc < flen; pc++) {
    - fentry = &filter[pc];
    + const struct sock_filter *fentry = &filter[pc];
    + u32 f_k = fentry->k;

    switch (fentry->code) {
    case BPF_S_ALU_ADD_X:
    A += X;
    continue;
    case BPF_S_ALU_ADD_K:
    - A += fentry->k;
    + A += f_k;
    continue;
    case BPF_S_ALU_SUB_X:
    A -= X;
    continue;
    case BPF_S_ALU_SUB_K:
    - A -= fentry->k;
    + A -= f_k;
    continue;
    case BPF_S_ALU_MUL_X:
    A *= X;
    continue;
    case BPF_S_ALU_MUL_K:
    - A *= fentry->k;
    + A *= f_k;
    continue;
    case BPF_S_ALU_DIV_X:
    if (X == 0)
    @@ -152,49 +154,49 @@ unsigned int sk_run_filter(struct sk_buf
    A /= X;
    continue;
    case BPF_S_ALU_DIV_K:
    - A /= fentry->k;
    + A /= f_k;
    continue;
    case BPF_S_ALU_AND_X:
    A &= X;
    continue;
    case BPF_S_ALU_AND_K:
    - A &= fentry->k;
    + A &= f_k;
    continue;
    case BPF_S_ALU_OR_X:
    A |= X;
    continue;
    case BPF_S_ALU_OR_K:
    - A |= fentry->k;
    + A |= f_k;
    continue;
    case BPF_S_ALU_LSH_X:
    A <<= X;
    continue;
    case BPF_S_ALU_LSH_K:
    - A <<= fentry->k;
    + A <<= f_k;
    continue;
    case BPF_S_ALU_RSH_X:
    A >>= X;
    continue;
    case BPF_S_ALU_RSH_K:
    - A >>= fentry->k;
    + A >>= f_k;
    continue;
    case BPF_S_ALU_NEG:
    A = -A;
    continue;
    case BPF_S_JMP_JA:
    - pc += fentry->k;
    + pc += f_k;
    continue;
    case BPF_S_JMP_JGT_K:
    - pc += (A > fentry->k) ? fentry->jt : fentry->jf;
    + pc += (A > f_k) ? fentry->jt : fentry->jf;
    continue;
    case BPF_S_JMP_JGE_K:
    - pc += (A >= fentry->k) ? fentry->jt : fentry->jf;
    + pc += (A >= f_k) ? fentry->jt : fentry->jf;
    continue;
    case BPF_S_JMP_JEQ_K:
    - pc += (A == fentry->k) ? fentry->jt : fentry->jf;
    + pc += (A == f_k) ? fentry->jt : fentry->jf;
    continue;
    case BPF_S_JMP_JSET_K:
    - pc += (A & fentry->k) ? fentry->jt : fentry->jf;
    + pc += (A & f_k) ? fentry->jt : fentry->jf;
    continue;
    case BPF_S_JMP_JGT_X:
    pc += (A > X) ? fentry->jt : fentry->jf;
    @@ -209,7 +211,7 @@ unsigned int sk_run_filter(struct sk_buf
    pc += (A & X) ? fentry->jt : fentry->jf;
    continue;
    case BPF_S_LD_W_ABS:
    - k = fentry->k;
    + k = f_k;
    load_w:
    ptr = load_pointer(skb, k, 4, &tmp);
    if (ptr != NULL) {
    @@ -218,7 +220,7 @@ load_w:
    }
    break;
    case BPF_S_LD_H_ABS:
    - k = fentry->k;
    + k = f_k;
    load_h:
    ptr = load_pointer(skb, k, 2, &tmp);
    if (ptr != NULL) {
    @@ -227,7 +229,7 @@ load_h:
    }
    break;
    case BPF_S_LD_B_ABS:
    - k = fentry->k;
    + k = f_k;
    load_b:
    ptr = load_pointer(skb, k, 1, &tmp);
    if (ptr != NULL) {
    @@ -242,32 +244,34 @@ load_b:
    X = skb->len;
    continue;
    case BPF_S_LD_W_IND:
    - k = X + fentry->k;
    + k = X + f_k;
    goto load_w;
    case BPF_S_LD_H_IND:
    - k = X + fentry->k;
    + k = X + f_k;
    goto load_h;
    case BPF_S_LD_B_IND:
    - k = X + fentry->k;
    + k = X + f_k;
    goto load_b;
    case BPF_S_LDX_B_MSH:
    - ptr = load_pointer(skb, fentry->k, 1, &tmp);
    + ptr = load_pointer(skb, f_k, 1, &tmp);
    if (ptr != NULL) {
    X = (*(u8 *)ptr & 0xf) << 2;
    continue;
    }
    return 0;
    case BPF_S_LD_IMM:
    - A = fentry->k;
    + A = f_k;
    continue;
    case BPF_S_LDX_IMM:
    - X = fentry->k;
    + X = f_k;
    continue;
    case BPF_S_LD_MEM:
    - A = mem[fentry->k];
    + A = (memvalid & (1UL << f_k)) ?
    + mem[f_k] : 0;
    continue;
    case BPF_S_LDX_MEM:
    - X = mem[fentry->k];
    + X = (memvalid & (1UL << f_k)) ?
    + mem[f_k] : 0;
    continue;
    case BPF_S_MISC_TAX:
    X = A;
    @@ -276,14 +280,16 @@ load_b:
    A = X;
    continue;
    case BPF_S_RET_K:
    - return fentry->k;
    + return f_k;
    case BPF_S_RET_A:
    return A;
    case BPF_S_ST:
    - mem[fentry->k] = A;
    + memvalid |= 1UL << f_k;
    + mem[f_k] = A;
    continue;
    case BPF_S_STX:
    - mem[fentry->k] = X;
    + memvalid |= 1UL << f_k;
    + mem[f_k] = X;
    continue;
    default:
    WARN_ON(1);



    \
     
     \ /
      Last update: 2010-12-08 02:13    [W:0.031 / U:0.736 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site