public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jann Horn <jannh@google.com>
Cc: stable@vger.kernel.org, security@kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>
Subject: Re: [PATCH for 4.1,4.4] bpf: fix incorrect sign extension in check_alu_op()
Date: Mon, 19 Mar 2018 18:13:12 +0100	[thread overview]
Message-ID: <20180319171312.GB22964@kroah.com> (raw)
In-Reply-To: <20180319165552.146891-1-jannh@google.com>

On Mon, Mar 19, 2018 at 05:55:52PM +0100, Jann Horn wrote:
> commit 95a762e2c8c942780948091f8f2a4f32fce1ac6f upstream.
> 
> Distinguish between
> BPF_ALU64|BPF_MOV|BPF_K (load 32-bit immediate, sign-extended to 64-bit)
> and BPF_ALU|BPF_MOV|BPF_K (load 32-bit immediate, zero-padded to 64-bit);
> only perform sign extension in the first case.
> 
> This patch differs from the mainline one because the verifier's internals
> have changed in the meantime. Mainline tracks register values as 64-bit
> values; however, 4.4 still stores tracked register values as 32-bit
> values with sign extension. Therefore, in the case of a 32-bit op with
> negative immediate, the value can't be tracked; leave the register as
> UNKNOWN_VALUE (set by the preceding check_reg_arg() call).
> 
> 
> I have manually tested this patch on top of 4.4.122. For the following BPF
> bytecode:
> 
>         BPF_MOV64_IMM(BPF_REG_1, 1),
>         BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
>         BPF_EXIT_INSN(),
> 
>         BPF_MOV32_IMM(BPF_REG_1, 1),
>         BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
>         BPF_EXIT_INSN(),
> 
>         BPF_MOV64_IMM(BPF_REG_1, -1),
>         BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, -1, 1),
>         BPF_EXIT_INSN(),
> 
>         BPF_MOV32_IMM(BPF_REG_1, -1),
>         BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, -1, 2),
>         BPF_MOV32_IMM(BPF_REG_0, 42),
>         BPF_EXIT_INSN(),
> 
>         BPF_MOV32_IMM(BPF_REG_0, 43),
>         BPF_EXIT_INSN()
> 
> Verifier output on 4.4.122 without this patch:
> 
> 0: (b7) r1 = 1
> 1: (15) if r1 == 0x1 goto pc+1
> 3: (b4) (u32) r1 = (u32) 1
> 4: (15) if r1 == 0x1 goto pc+1
> 6: (b7) r1 = -1
> 7: (15) if r1 == 0xffffffff goto pc+1
> 9: (b4) (u32) r1 = (u32) -1
> 10: (15) if r1 == 0xffffffff goto pc+2
> 13: (b4) (u32) r0 = (u32) 43
> 14: (95) exit
> 
> Verifier output on 4.4.122+ with this patch:
> 
> 0: (b7) r1 = 1
> 1: (15) if r1 == 0x1 goto pc+1
> 3: (b4) (u32) r1 = (u32) 1
> 4: (15) if r1 == 0x1 goto pc+1
> 6: (b7) r1 = -1
> 7: (15) if r1 == 0xffffffff goto pc+1
> 9: (b4) (u32) r1 = (u32) -1
> 10: (15) if r1 == 0xffffffff goto pc+2
>  R1=inv R10=fp
> 11: (b4) (u32) r0 = (u32) 42
> 12: (95) exit
> 
> from 10 to 13: R1=imm-1 R10=fp
> 13: (b4) (u32) r0 = (u32) 43
> 14: (95) exit
> 
> 
> This patch should be applied to linux-4.4.y and linux-4.1.y.

Now queued up for 4.4.y, thanks!

greg k-h

      parent reply	other threads:[~2018-03-19 17:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-18  2:17 requesting stable backport for 4.1 and 4.4: 95a762e2c8c9 ("bpf: fix incorrect sign extension in check_alu_op()") Jann Horn
2018-03-18 12:57 ` Greg Kroah-Hartman
2018-03-19 16:55 ` [PATCH for 4.1,4.4] bpf: fix incorrect sign extension in check_alu_op() Jann Horn
2018-03-19 16:58   ` Daniel Borkmann
2018-03-19 17:13   ` Patch "bpf: fix incorrect sign extension in check_alu_op()" has been added to the 4.4-stable tree gregkh
2018-03-19 17:13   ` Greg Kroah-Hartman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180319171312.GB22964@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jannh@google.com \
    --cc=security@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox