[PATCH v4.4.y] bpf: reject wrong sized filters earlier

[PATCH v4.4.y] bpf: reject wrong sized filters earlier

[Zubin Mithra]

From: Daniel Borkmann <daniel@iogearbox.net>

commit f7bd9e36ee4a4ce38e1cddd7effe6c0d9943285b upstream

Add a bpf_check_basics_ok() and reject filters that are of invalid
size much earlier, so we don't do any useless work such as invoking
bpf_prog_alloc(). Currently, rejection happens in bpf_check_classic()
only, but it's really unnecessarily late and they should be rejected
at earliest point. While at it, also clean up one bpf_prog_size() to
make it consistent with the remaining invocations.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Zubin Mithra <zsm@chromium.org>
---
Notes:
* Syzkaller reported a kernel BUG related to a kernel paging request in
  bpf_prog_create with the following stacktrace when fuzzing a 4.4 kernel.
Call Trace:
 [<ffffffff822ac1c8>] bpf_prog_create+0xc8/0x210 net/core/filter.c:1067
 [<ffffffff82454699>] bpf_mt_check+0xb9/0x120 net/netfilter/xt_bpf.c:31
 [<ffffffff82437db8>] xt_check_match+0x238/0x730 net/netfilter/x_tables.c:409
 [<ffffffff82940254>] ebt_check_match net/bridge/netfilter/ebtables.c:380 [inline]
 [<ffffffff82940254>] ebt_check_entry+0x844/0x1740 net/bridge/netfilter/ebtables.c:709
 [<ffffffff82946842>] translate_table+0xcb2/0x1e80 net/bridge/netfilter/ebtables.c:946
 [<ffffffff8294a918>] do_replace_finish+0x6e8/0x1fd0 net/bridge/netfilter/ebtables.c:1002
 [<ffffffff8294c419>] do_replace+0x219/0x370 net/bridge/netfilter/ebtables.c:1145
 [<ffffffff8294c649>] do_ebt_set_ctl+0xd9/0x110 net/bridge/netfilter/ebtables.c:1492
 [<ffffffff8239a87c>] nf_sockopt net/netfilter/nf_sockopt.c:105 [inline]
 [<ffffffff8239a87c>] nf_setsockopt+0x6c/0xc0 net/netfilter/nf_sockopt.c:114
 [<ffffffff825ddeb6>] ip_setsockopt+0xa6/0xc0 net/ipv4/ip_sockglue.c:1226
 [<ffffffff825fd3c7>] tcp_setsockopt+0x87/0xd0 net/ipv4/tcp.c:2701
 [<ffffffff8220343a>] sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:2690
 [<ffffffff822006ed>] SYSC_setsockopt net/socket.c:1767 [inline]
 [<ffffffff822006ed>] SyS_setsockopt+0x15d/0x240 net/socket.c:1746
 [<ffffffff82a16f9b>] entry_SYSCALL_64_fastpath+0x18/0x94

* This patch resolves the following conflicts when applying to v4.4.y:
- __get_filter does not exist in v4.4. Instead the checks are moved into
  __sk_attach_filter.

* This patch is present in v4.9.y.

* Tests run: Chrome OS tryjobs, Syzkaller reproducer

 net/core/filter.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 1a9ded6af138..3c5f51198c41 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -742,6 +742,17 @@ static bool chk_code_allowed(u16 code_to_probe)
 	return codes[code_to_probe];
 }
 
+static bool bpf_check_basics_ok(const struct sock_filter *filter,
+				unsigned int flen)
+{
+	if (filter == NULL)
+		return false;
+	if (flen == 0 || flen > BPF_MAXINSNS)
+		return false;
+
+	return true;
+}
+
 /**
  *	bpf_check_classic - verify socket filter code
  *	@filter: filter to verify
@@ -762,9 +773,6 @@ static int bpf_check_classic(const struct sock_filter *filter,
 	bool anc_found;
 	int pc;
 
-	if (flen == 0 || flen > BPF_MAXINSNS)
-		return -EINVAL;
-
 	/* Check the filter code now */
 	for (pc = 0; pc < flen; pc++) {
 		const struct sock_filter *ftest = &filter[pc];
@@ -1057,7 +1065,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog)
 	struct bpf_prog *fp;
 
 	/* Make sure new filter is there and in the right amounts. */
-	if (fprog->filter == NULL)
+	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return -EINVAL;
 
 	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
@@ -1104,7 +1112,7 @@ int bpf_prog_create_from_user(struct bpf_prog **pfp, struct sock_fprog *fprog,
 	int err;
 
 	/* Make sure new filter is there and in the right amounts. */
-	if (fprog->filter == NULL)
+	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return -EINVAL;
 
 	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
@@ -1184,7 +1192,6 @@ int __sk_attach_filter(struct sock_fprog *fprog, struct sock *sk,
 		       bool locked)
 {
 	unsigned int fsize = bpf_classic_proglen(fprog);
-	unsigned int bpf_fsize = bpf_prog_size(fprog->len);
 	struct bpf_prog *prog;
 	int err;
 
@@ -1192,10 +1199,10 @@ int __sk_attach_filter(struct sock_fprog *fprog, struct sock *sk,
 		return -EPERM;
 
 	/* Make sure new filter is there and in the right amounts. */
-	if (fprog->filter == NULL)
+	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return -EINVAL;
 
-	prog = bpf_prog_alloc(bpf_fsize, 0);
+	prog = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
 	if (!prog)
 		return -ENOMEM;
 
-- 
2.21.0.593.g511ec345e18-goog

Re: [PATCH v4.4.y] bpf: reject wrong sized filters earlier

[Greg KH]

On Wed, Apr 24, 2019 at 11:00:18AM -0700, Zubin Mithra wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> 
> commit f7bd9e36ee4a4ce38e1cddd7effe6c0d9943285b upstream
> 
> Add a bpf_check_basics_ok() and reject filters that are of invalid
> size much earlier, so we don't do any useless work such as invoking
> bpf_prog_alloc(). Currently, rejection happens in bpf_check_classic()
> only, but it's really unnecessarily late and they should be rejected
> at earliest point. While at it, also clean up one bpf_prog_size() to
> make it consistent with the remaining invocations.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Zubin Mithra <zsm@chromium.org>
> ---
> Notes:
> * Syzkaller reported a kernel BUG related to a kernel paging request in
>   bpf_prog_create with the following stacktrace when fuzzing a 4.4 kernel.
> Call Trace:
>  [<ffffffff822ac1c8>] bpf_prog_create+0xc8/0x210 net/core/filter.c:1067
>  [<ffffffff82454699>] bpf_mt_check+0xb9/0x120 net/netfilter/xt_bpf.c:31
>  [<ffffffff82437db8>] xt_check_match+0x238/0x730 net/netfilter/x_tables.c:409
>  [<ffffffff82940254>] ebt_check_match net/bridge/netfilter/ebtables.c:380 [inline]
>  [<ffffffff82940254>] ebt_check_entry+0x844/0x1740 net/bridge/netfilter/ebtables.c:709
>  [<ffffffff82946842>] translate_table+0xcb2/0x1e80 net/bridge/netfilter/ebtables.c:946
>  [<ffffffff8294a918>] do_replace_finish+0x6e8/0x1fd0 net/bridge/netfilter/ebtables.c:1002
>  [<ffffffff8294c419>] do_replace+0x219/0x370 net/bridge/netfilter/ebtables.c:1145
>  [<ffffffff8294c649>] do_ebt_set_ctl+0xd9/0x110 net/bridge/netfilter/ebtables.c:1492
>  [<ffffffff8239a87c>] nf_sockopt net/netfilter/nf_sockopt.c:105 [inline]
>  [<ffffffff8239a87c>] nf_setsockopt+0x6c/0xc0 net/netfilter/nf_sockopt.c:114
>  [<ffffffff825ddeb6>] ip_setsockopt+0xa6/0xc0 net/ipv4/ip_sockglue.c:1226
>  [<ffffffff825fd3c7>] tcp_setsockopt+0x87/0xd0 net/ipv4/tcp.c:2701
>  [<ffffffff8220343a>] sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:2690
>  [<ffffffff822006ed>] SYSC_setsockopt net/socket.c:1767 [inline]
>  [<ffffffff822006ed>] SyS_setsockopt+0x15d/0x240 net/socket.c:1746
>  [<ffffffff82a16f9b>] entry_SYSCALL_64_fastpath+0x18/0x94
> 
> * This patch resolves the following conflicts when applying to v4.4.y:
> - __get_filter does not exist in v4.4. Instead the checks are moved into
>   __sk_attach_filter.
> 
> * This patch is present in v4.9.y.
> 
> * Tests run: Chrome OS tryjobs, Syzkaller reproducer

Now queued up, thanks.

greg k-h