Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down
Alexei Starovoitov 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
Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down
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 > > 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 > Signed-off-by: David Howells > cc: net...@vger.kernel.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
On 6 April 2017 at 13:29, Alexei Starovoitov
wrote:
> On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
>> From: Chun-Yi Lee
>>
>> 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
>> Signed-off-by: David Howells
>> cc: net...@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
On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
> From: Chun-Yi Lee
>
> 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
> Signed-off-by: David Howells
> cc: net...@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.
[PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down
From: Chun-Yi Lee
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
Signed-off-by: David Howells
cc: net...@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]))
