diff mbox series

[5.10,39/41] bpf, x86: Validate computation of branch displacements for x86-32

Message ID 20210409095306.075652415@linuxfoundation.org
State Superseded
Headers show
Series None | expand

Commit Message

Greg Kroah-Hartman April 9, 2021, 9:54 a.m. UTC 
From: Piotr Krysiuk <piotras@gmail.com>

commit 26f55a59dc65ff77cd1c4b37991e26497fc68049 upstream.

The branch displacement logic in the BPF JIT compilers for x86 assumes
that, for any generated branch instruction, the distance cannot
increase between optimization passes.

But this assumption can be violated due to how the distances are
computed. Specifically, whenever a backward branch is processed in
do_jit(), the distance is computed by subtracting the positions in the
machine code from different optimization passes. This is because part
of addrs[] is already updated for the current optimization pass, before
the branch instruction is visited.

And so the optimizer can expand blocks of machine code in some cases.

This can confuse the optimizer logic, where it assumes that a fixed
point has been reached for all machine code blocks once the total
program size stops changing. And then the JIT compiler can output
abnormal machine code containing incorrect branch displacements.

To mitigate this issue, we assert that a fixed point is reached while
populating the output image. This rejects any problematic programs.
The issue affects both x86-32 and x86-64. We mitigate separately to
ease backporting.

Signed-off-by: Piotr Krysiuk <piotras@gmail.com>
Reviewed-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/x86/net/bpf_jit_comp32.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Sudip Mukherjee April 9, 2021, 7:51 p.m. UTC | #1 
Hi Greg,

On Fri, Apr 09, 2021 at 11:54:01AM +0200, Greg Kroah-Hartman wrote:
> From: Piotr Krysiuk <piotras@gmail.com>
> 
> commit 26f55a59dc65ff77cd1c4b37991e26497fc68049 upstream.

I am not finding this in Linus's tree and even not seeing this change in
master branch also. Am I missing something?


--
Regards
Sudip
Daniel Borkmann April 9, 2021, 8:13 p.m. UTC | #2 
On 4/9/21 9:51 PM, Sudip Mukherjee wrote:
> On Fri, Apr 09, 2021 at 11:54:01AM +0200, Greg Kroah-Hartman wrote:
>> From: Piotr Krysiuk <piotras@gmail.com>
>>
>> commit 26f55a59dc65ff77cd1c4b37991e26497fc68049 upstream.
> 
> I am not finding this in Linus's tree and even not seeing this change in
> master branch also. Am I missing something?

Both are in -net tree at this point, thus commit sha won't change anymore. David or
Jakub will likely send their -net PR to Linus today or tomorrow for landing in
mainline. For stable things had to move a bit quicker given the announcement in [0]
yesterday. Timing was a bit unfortunate here as I would have preferred for things to
land in stable the regular way first (aka merge to mainline, cherry-picking to stable,
minor stable release, then announcement).

Thanks,
Daniel

   [0] https://www.openwall.com/lists/oss-security/2021/04/08/1
diff mbox series

Patch

--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -2278,7 +2278,16 @@  notyet:
 		}
 
 		if (image) {
-			if (unlikely(proglen + ilen > oldproglen)) {
+			/*
+			 * When populating the image, assert that:
+			 *
+			 *  i) We do not write beyond the allocated space, and
+			 * ii) addrs[i] did not change from the prior run, in order
+			 *     to validate assumptions made for computing branch
+			 *     displacements.
+			 */
+			if (unlikely(proglen + ilen > oldproglen ||
+				     proglen + ilen != addrs[i])) {
 				pr_err("bpf_jit: fatal error\n");
 				return -EFAULT;
 			}