[PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[David Howells]

From: Chun-Yi Lee <jlee@suse.com>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program.  Prohibit those functions when the kernel is
locked down.

Signed-off-by: Chun-Yi Lee <jlee@suse.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: netdev@vger.kernel.org
---

 kernel/trace/bpf_trace.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cee9802cf3e0..7fde851f207b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
 {
 	int ret;
 
+	if (kernel_is_locked_down()) {
+		memset(dst, 0, size);
+		return -EPERM;
+	}
+
 	ret = probe_kernel_read(dst, unsafe_ptr, size);
 	if (unlikely(ret < 0))
 		memset(dst, 0, size);
@@ -84,6 +89,9 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
 	   u32, size)
 {
+	if (kernel_is_locked_down())
+		return -EPERM;
+
 	/*
 	 * Ensure we're in user context which is safe for the helper to
 	 * run. This helper has no business in a kthread.
@@ -143,6 +151,9 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	if (fmt[--fmt_size] != 0)
 		return -EINVAL;
 
+	if (kernel_is_locked_down())
+		return __trace_printk(1, fmt, 0, 0, 0);
+
 	/* check format string for allowed specifiers */
 	for (i = 0; i < fmt_size; i++) {
 		if ((!isprint(fmt[i]) && !isspace(fmt[i])) || !isascii(fmt[i]))

Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[Alexei Starovoitov]

On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
> From: Chun-Yi Lee <jlee@suse.com>
> 
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program.  Prohibit those functions when the kernel is
> locked down.
> 
> Signed-off-by: Chun-Yi Lee <jlee@suse.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: netdev@vger.kernel.org
> ---
> 
>  kernel/trace/bpf_trace.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index cee9802cf3e0..7fde851f207b 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
>  {
>  	int ret;
>  
> +	if (kernel_is_locked_down()) {
> +		memset(dst, 0, size);
> +		return -EPERM;
> +	}

this will obviously break the program. How about disabling loading tracing
programs during the lockdown completely?

Also is there a description of what this lockdown trying to accomplish?
The cover letter is scarce in details.

Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[Ard Biesheuvel]

On 6 April 2017 at 13:29, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
>> From: Chun-Yi Lee <jlee@suse.com>
>>
>> There are some bpf functions can be used to read kernel memory:
>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
>> private keys in kernel memory (e.g. the hibernation image signing key) to
>> be read by an eBPF program.  Prohibit those functions when the kernel is
>> locked down.
>>
>> Signed-off-by: Chun-Yi Lee <jlee@suse.com>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> cc: netdev@vger.kernel.org
>> ---
>>
>>  kernel/trace/bpf_trace.c |   11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index cee9802cf3e0..7fde851f207b 100644
>> --- a/kernel/trace/bpf_trace.c
>> +++ b/kernel/trace/bpf_trace.c
>> @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
>>  {
>>       int ret;
>>
>> +     if (kernel_is_locked_down()) {
>> +             memset(dst, 0, size);
>> +             return -EPERM;
>> +     }
>
> this will obviously break the program. How about disabling loading tracing
> programs during the lockdown completely?
>
> Also is there a description of what this lockdown trying to accomplish?
> The cover letter is scarce in details.
>

This is a very good point, and this is actually feedback that was
given (by Alan Cox, iirc) the last time this series was circulated.

This series is a mixed bag of patches that all look like they improve
'security' in one way or the other. But what is lacking is a coherent
view on the threat model, and to what extent all these patches reduce
the vulnerability to such threats. Without that, these patches do very
little beyond giving a false sense of security, imo.

Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[joeyli]

Hi David,

First, thanks for your help to send out this series.

On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
> From: Chun-Yi Lee <jlee-IBi9RG/b67k@public.gmane.org>
> 
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program.  Prohibit those functions when the kernel is
> locked down.
> 
> Signed-off-by: Chun-Yi Lee <jlee-IBi9RG/b67k@public.gmane.org>
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

This patch is used with hibernation signature verification. I suggest
that we can remove this patch from your series because we just lock
down the hibernation function until hibernation verification get
accepted.

On the other hand, we are trying to enhance the bpf verifier to
prevent bpf print reads specific memory addresses that have sensitive
data.

Thanks a lot!
Joey Lee

Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

[David Howells]

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> this will obviously break the program.

Yeah.  But if it allows one to twiddle the kernel image or gain access to
crypto material...

> How about disabling loading tracing programs during the lockdown completely?

Interesting thought.  I'm not sure how much would actually need locking down
here.  Turning on tracepoints in the kernel and reading out of the trace
buffer, for example, ought to be okay, though if there are any tracepoints
that leak crypto information, they may need locking down also.

Basically, I think it boils down to: if it can be used to modify the kernel
image or read arbitrary data from the kernel image then should probably be
forbidden.  There have to be exceptions, though, such as loading authenticated
kernel modules.

David