diff mbox series

[net-next] net: filter: remove dead instructions in filter code

Message ID 525d54bc-5259-49f2-acbf-7396bab48dec@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: filter: remove dead instructions in filter code | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 13 maintainers not CCed: yonghong.song@linux.dev andrii@kernel.org ast@kernel.org netdev@vger.kernel.org john.fastabend@gmail.com song@kernel.org eddyz87@gmail.com sdf@fomichev.me martin.lau@linux.dev haoluo@google.com jolsa@kernel.org horms@kernel.org kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 26 this patch: 26
netdev/checkpatch warning WARNING: From:/Signed-off-by: email name mismatch: 'From: Lion Ackermann <nnamrec@gmail.com>' != 'Signed-off-by: Lion <nnamrec@gmail.com>'
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 6 this patch: 6
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2025-04-10--12-00 (tests: 900)

Commit Message

Lion Ackermann April 10, 2025, 8:32 a.m. UTC
It is well-known to be possible to abuse the eBPF JIT to construct
gadgets for code re-use attacks. To hinder this constant blinding was
added in "bpf: add generic constant blinding for use in jits". This
mitigation has one weakness though: It ignores jump instructions due to
their correct offsets not being known when constant blinding is applied.
This can be abused to construct "jump-chains" with crafted offsets so
that certain desirable instructions are generated by the JIT compiler.
F.e. two consecutive BPF_JMP | BPF_JA codes with an appropriate offset
might generate the following jumps:

    ...
    0xffffffffc000f822:    jmp    0xffffffffc00108df
    0xffffffffc000f827:    jmp    0xffffffffc0010861
    ...

If those are hit unaligned we can get two consecutive useful
instructions:

    ...
    0xffffffffc000f823:    mov    $0xe9000010,%eax
    0xffffffffc000f828:    xor    $0xe9000010,%eax
    ...

This patch adds a mitigation to prevent said chains from being
generated by re-writing any instructions which are not reachable
anyways. By preventing consecutive jumps, only a single instruction can
be encoded, which is believed to be insufficient to be useful.

No functional changes for a benign filter program are intended.

Fixes: 4f3446bb809f ("bpf: add generic constant blinding for use in jits")
Signed-off-by: Lion <nnamrec@gmail.com>
---
 net/core/filter.c | 68 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 3 deletions(-)

Comments

Alexei Starovoitov April 10, 2025, 3:05 p.m. UTC | #1
On Thu, Apr 10, 2025 at 1:32 AM Lion Ackermann <nnamrec@gmail.com> wrote:
>
> It is well-known to be possible to abuse the eBPF JIT to construct
> gadgets for code re-use attacks. To hinder this constant blinding was
> added in "bpf: add generic constant blinding for use in jits". This
> mitigation has one weakness though: It ignores jump instructions due to
> their correct offsets not being known when constant blinding is applied.
> This can be abused to construct "jump-chains" with crafted offsets so
> that certain desirable instructions are generated by the JIT compiler.
> F.e. two consecutive BPF_JMP | BPF_JA codes with an appropriate offset
> might generate the following jumps:
>
>     ...
>     0xffffffffc000f822:    jmp    0xffffffffc00108df
>     0xffffffffc000f827:    jmp    0xffffffffc0010861
>     ...
>
> If those are hit unaligned we can get two consecutive useful
> instructions:
>
>     ...
>     0xffffffffc000f823:    mov    $0xe9000010,%eax
>     0xffffffffc000f828:    xor    $0xe9000010,%eax
>     ...

Nack.
This is not exploitable.
We're not going to complicate classic bpf because of theoretical concerns.

pw-bot: cr
Lion Ackermann April 11, 2025, 6:24 a.m. UTC | #2
On 4/10/25 5:05 PM, Alexei Starovoitov wrote:
> On Thu, Apr 10, 2025 at 1:32 AM Lion Ackermann <nnamrec@gmail.com> wrote:
>>
>> It is well-known to be possible to abuse the eBPF JIT to construct
>> gadgets for code re-use attacks. To hinder this constant blinding was
>> added in "bpf: add generic constant blinding for use in jits". This
>> mitigation has one weakness though: It ignores jump instructions due to
>> their correct offsets not being known when constant blinding is applied.
>> This can be abused to construct "jump-chains" with crafted offsets so
>> that certain desirable instructions are generated by the JIT compiler.
>> F.e. two consecutive BPF_JMP | BPF_JA codes with an appropriate offset
>> might generate the following jumps:
>>
>>     ...
>>     0xffffffffc000f822:    jmp    0xffffffffc00108df
>>     0xffffffffc000f827:    jmp    0xffffffffc0010861
>>     ...
>>
>> If those are hit unaligned we can get two consecutive useful
>> instructions:
>>
>>     ...
>>     0xffffffffc000f823:    mov    $0xe9000010,%eax
>>     0xffffffffc000f828:    xor    $0xe9000010,%eax
>>     ...
> 
> Nack.
> This is not exploitable.
> We're not going to complicate classic bpf because of theoretical concerns.
> 
> pw-bot: cr

This is not a theoretical concern, it is actually very practical. Sorry
for not making this clearer. I would rather not share full payloads
publicly at this point, though.
I understand that it is undesirable to complicate the code, but after
some initial discussion this seemed to be the least intrusive option.
However I would appreciate suggestions for better solutions..

Thanks,
Lion
Alexei Starovoitov April 11, 2025, 3:20 p.m. UTC | #3
On Thu, Apr 10, 2025 at 11:24 PM Lion Ackermann <nnamrec@gmail.com> wrote:
>
> On 4/10/25 5:05 PM, Alexei Starovoitov wrote:
> > On Thu, Apr 10, 2025 at 1:32 AM Lion Ackermann <nnamrec@gmail.com> wrote:
> >>
> >> It is well-known to be possible to abuse the eBPF JIT to construct
> >> gadgets for code re-use attacks. To hinder this constant blinding was
> >> added in "bpf: add generic constant blinding for use in jits". This
> >> mitigation has one weakness though: It ignores jump instructions due to
> >> their correct offsets not being known when constant blinding is applied.
> >> This can be abused to construct "jump-chains" with crafted offsets so
> >> that certain desirable instructions are generated by the JIT compiler.
> >> F.e. two consecutive BPF_JMP | BPF_JA codes with an appropriate offset
> >> might generate the following jumps:
> >>
> >>     ...
> >>     0xffffffffc000f822:    jmp    0xffffffffc00108df
> >>     0xffffffffc000f827:    jmp    0xffffffffc0010861
> >>     ...
> >>
> >> If those are hit unaligned we can get two consecutive useful
> >> instructions:
> >>
> >>     ...
> >>     0xffffffffc000f823:    mov    $0xe9000010,%eax
> >>     0xffffffffc000f828:    xor    $0xe9000010,%eax
> >>     ...
> >
> > Nack.
> > This is not exploitable.
> > We're not going to complicate classic bpf because of theoretical concerns.
> >
> > pw-bot: cr
>
> This is not a theoretical concern, it is actually very practical. Sorry
> for not making this clearer. I would rather not share full payloads
> publicly at this point, though.

Do share.
JIT spraying is nothing new. Blinding only made it harder.
There are lots of usable gadgets without it as well.
Turn off JIT completely and nothing changes from security pov.
Lion Ackermann April 15, 2025, 8:54 a.m. UTC | #4
On 4/11/25 5:20 PM, Alexei Starovoitov wrote:
> On Thu, Apr 10, 2025 at 11:24 PM Lion Ackermann <nnamrec@gmail.com> wrote:
>>
>> On 4/10/25 5:05 PM, Alexei Starovoitov wrote:
>>> On Thu, Apr 10, 2025 at 1:32 AM Lion Ackermann <nnamrec@gmail.com> wrote:
>>>>
>>>> It is well-known to be possible to abuse the eBPF JIT to construct
>>>> gadgets for code re-use attacks. To hinder this constant blinding was
>>>> added in "bpf: add generic constant blinding for use in jits". This
>>>> mitigation has one weakness though: It ignores jump instructions due to
>>>> their correct offsets not being known when constant blinding is applied.
>>>> This can be abused to construct "jump-chains" with crafted offsets so
>>>> that certain desirable instructions are generated by the JIT compiler.
>>>> F.e. two consecutive BPF_JMP | BPF_JA codes with an appropriate offset
>>>> might generate the following jumps:
>>>>
>>>>     ...
>>>>     0xffffffffc000f822:    jmp    0xffffffffc00108df
>>>>     0xffffffffc000f827:    jmp    0xffffffffc0010861
>>>>     ...
>>>>
>>>> If those are hit unaligned we can get two consecutive useful
>>>> instructions:
>>>>
>>>>     ...
>>>>     0xffffffffc000f823:    mov    $0xe9000010,%eax
>>>>     0xffffffffc000f828:    xor    $0xe9000010,%eax
>>>>     ...
>>>
>>> Nack.
>>> This is not exploitable.
>>> We're not going to complicate classic bpf because of theoretical concerns.
>>>
>>> pw-bot: cr
>>
>> This is not a theoretical concern, it is actually very practical. Sorry
>> for not making this clearer. I would rather not share full payloads
>> publicly at this point, though.
> 
> Do share.

I am not sure if sharing adds any particular value here. The mitigation
targets the 5-byte-variant of the x86 jmp instruction as stated above.
You would only get more examples of the same instruction.
Also note that the mitigation does not prevent the 2-byte-variant.
Beside jumps, there are a couple of other possible instructions that
allow a 1 byte payload encoding, so I did not bother.

> JIT spraying is nothing new. Blinding only made it harder.
> There are lots of usable gadgets without it as well.
> Turn off JIT completely and nothing changes from security pov.

True, the proposal only has an effect if blinding+jit is enabled. 
Otherwise it's useless.
If that is not good enough to add complexity to the cBPF code, then
I suppose we should take this back to the drawing board?

Thanks,
Lion
diff mbox series

Patch

diff --git a/net/core/filter.c b/net/core/filter.c
index bc6828761a47..b8eb2fa309c6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -970,6 +970,65 @@  static int check_load_and_stores(const struct sock_filter *filter, int flen)
 	return ret;
 }
 
+/* Security:
+ *
+ * As it is possible to abuse the JIT compiler to produce instructions for
+ * code re-use, remove anything which is not reachable in a benign program
+ * anyways
+ */
+static int remove_dead_code(struct sock_filter *filter, int flen)
+{
+	int pc;
+	unsigned long *live;
+
+	if (flen == 0)
+		return 0;
+
+	live = bitmap_zalloc(flen, GFP_KERNEL);
+	if (!live)
+		return -ENOMEM;
+
+	/* No back jumps and no loops, can do a single forward pass here */
+	set_bit(0, live);
+	for (pc = 0; pc < flen; pc++) {
+		if (!test_bit(pc, live)) {
+			/* Dead. Some arbitrary instruction here. */
+			filter[pc].code = BPF_LD | BPF_IMM;
+			filter[pc].k = 0;
+			filter[pc].jt = 0;
+			filter[pc].jf = 0;
+			continue;
+		}
+
+		switch (filter[pc].code) {
+		case BPF_RET | BPF_K:
+		case BPF_RET | BPF_A:
+			break;
+		case BPF_JMP | BPF_JA:
+			set_bit(pc + 1 + filter[pc].k, live);
+			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:
+			set_bit(pc + 1 + filter[pc].jt, live);
+			set_bit(pc + 1 + filter[pc].jf, live);
+			break;
+		default:
+			/* Continue to next instruction */
+			set_bit(pc + 1, live);
+			break;
+		}
+	}
+
+	kfree(live);
+	return 0;
+}
+
 static bool chk_code_allowed(u16 code_to_probe)
 {
 	static const bool codes[] = {
@@ -1061,11 +1120,11 @@  static bool bpf_check_basics_ok(const struct sock_filter *filter,
  *
  * Returns 0 if the rule set is legal or -EINVAL if not.
  */
-static int bpf_check_classic(const struct sock_filter *filter,
+static int bpf_check_classic(struct sock_filter *filter,
 			     unsigned int flen)
 {
 	bool anc_found;
-	int pc;
+	int pc, ret;
 
 	/* Check the filter code now */
 	for (pc = 0; pc < flen; pc++) {
@@ -1133,7 +1192,10 @@  static int bpf_check_classic(const struct sock_filter *filter,
 	switch (filter[flen - 1].code) {
 	case BPF_RET | BPF_K:
 	case BPF_RET | BPF_A:
-		return check_load_and_stores(filter, flen);
+		ret = check_load_and_stores(filter, flen);
+		if (ret)
+			return ret;
+		return remove_dead_code(filter, flen);
 	}
 
 	return -EINVAL;