linux-kernel - Re: [PATCH v12 01/13] sk_run_filter: add support for custom load

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 2 Mar 2012 11:40:14 +0100
From:	"Indan Zupancic" <indan@....nu>
To:	"Will Drewry" <wad@...omium.org>
Cc:	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-doc@...r.kernel.org, kernel-hardening@...ts.openwall.com,
	netdev@...r.kernel.org, x86@...nel.org, arnd@...db.de,
	davem@...emloft.net, hpa@...or.com, mingo@...hat.com,
	oleg@...hat.com, peterz@...radead.org, rdunlap@...otime.net,
	mcgrathr@...omium.org, tglx@...utronix.de, luto@....edu,
	eparis@...hat.com, serge.hallyn@...onical.com, djm@...drot.org,
	scarybeasts@...il.com, pmoore@...hat.com,
	akpm@...ux-foundation.org, corbet@....net, eric.dumazet@...il.com,
	markus@...omium.org, coreyb@...ux.vnet.ibm.com,
	keescook@...omium.org, "Will Drewry" <wad@...omium.org>
Subject: Re: [PATCH v12 01/13] sk_run_filter: add support for custom
 load_pointer

Hello,

On Thu, March 1, 2012 00:53, Will Drewry wrote:
>  include/linux/filter.h |   46 +++++++++++++++++++
>  net/core/filter.c      |  117 +++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 157 insertions(+), 6 deletions(-)

I propose a slightly different approach:

Instead of more or less allowing generic load instructions, do the
same as the ancillary data functions and only allow BPF_S_LD_W_ABS.
In addition to that, rewrite and check the functions ourself after
sk_chk_filter() has done its checks.

Diff for filter.c:

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 8eeb205..63b728c 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -228,6 +228,7 @@ enum {
 	BPF_S_ANC_HATYPE,
 	BPF_S_ANC_RXHASH,
 	BPF_S_ANC_CPU,
+	BPF_S_LD_W_SECCOMP,
 };

 #endif /* __KERNEL__ */
diff --git a/net/core/filter.c b/net/core/filter.c
index 5dea452..7e338d6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -350,6 +350,9 @@ load_b:
 				A = 0;
 			continue;
 		}
+		case BPF_S_LD_W_SECCOMP:
+			A = seccomp_load(fentry->k);
+			continue;
 		default:
 			WARN_RATELIMIT(1, "Unknown code:%u jt:%u tf:%u k:%u\n",
 				       fentry->code, fentry->jt,
---

And in seccomp add something like:

/*
 * Does SECCOMP specific checks.
 * Should be called after sk_chk_filter(), as it assumes all instructions
 * are rewritten to the kernel enum format.
 * No SKB touching instructions are allowed. Only data LD instruction allowed
 * is BPF_S_LD_W_ABS, which will be handled by seccomp_load().
 */
int seccomp_check_filter(const struct sock_filter *filter, unsigned int flen)
{
	int pc;

	/* Make sure there are no SKB using instructions */
	for (pc = 0; pc < flen; pc++) {
		u16 code = filter->code;
		unsigned int k = filter->k;

		if (code <= BPF_S_ALU_NEG)
			continue;
		if (code >= BPF_S_LDX_IMM && code < BPF_S_ANC_PROTOCOL)
			continue;
		switch (code) {
		case BPF_S_LD_W_ABS:
			filter->code = BPF_S_LD_W_SECCOMP;
			if (k >= sizeof(struct seccomp_data) || k & 3)
				return -EINVAL;
			continue;
		case BPF_S_LD_W_LEN:
			filter->code = BPF_S_LD_IMM;
			filter->k = sizeof(struct seccomp_data);
			continue;
		case BPF_S_LD_IMM:
			continue;
		case BPF_S_LDX_W_LEN:
			filter->code = BPF_S_LDX_IMM;
			filter->k = sizeof(struct seccomp_data);
			continue;
		default:
			return -EINVAL;
		}
	}
	return 0;
}

u32 seccomp_load(int off)
{
	u32 A;
	struct pt_regs *regs = task_pt_regs(current);

	if (off >= BPF_DATA(args[0]) && off < BPF_DATA(args[6])) {
		int arg = (off - BPF_DATA(args[0])) / sizeof(u64);
		int index = (off % sizeof(u64)) ? 1 : 0;
		syscall_get_arguments(current, regs, arg, 1, &value);
		A = get_u32(value, index);
	} else if (off == BPF_DATA(nr)) {
		A = syscall_get_nr(current, regs);
	} else if (off == BPF_DATA(arch)) {
		A = syscall_get_arch(current, regs);
	} else if (off == BPF_DATA(instruction_pointer)) {
		A = get_u32(KSTK_EIP(current), 0);
	} else if (off == BPF_DATA(instruction_pointer) + sizeof(u32)) {
		A = get_u32(KSTK_EIP(current), 1);
	}
	return A;
}

This way you can even add SECCOMP specific functions in the future by using
special offsets. (E.g. 64-bit compare between an arg and scratch memory.)

Greetings,

Indan


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/