From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751819AbbJEVce (ORCPT ); Mon, 5 Oct 2015 17:32:34 -0400 Received: from mail-pa0-f42.google.com ([209.85.220.42]:34837 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751220AbbJEVcb (ORCPT ); Mon, 5 Oct 2015 17:32:31 -0400 Subject: Re: [PATCH net-next 1/2] bpf: enable non-root eBPF programs To: Andy Lutomirski References: <1444078101-29060-1-git-send-email-ast@plumgrid.com> <1444078101-29060-2-git-send-email-ast@plumgrid.com> <5612E7C4.1010306@plumgrid.com> Cc: Kees Cook , "David S. Miller" , Ingo Molnar , Hannes Frederic Sowa , Eric Dumazet , Daniel Borkmann , Linux API , Network Development , LKML From: Alexei Starovoitov Message-ID: <5612EC6D.6080301@plumgrid.com> Date: Mon, 5 Oct 2015 14:32:29 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/5/15 2:16 PM, Andy Lutomirski wrote: > On Mon, Oct 5, 2015 at 2:12 PM, Alexei Starovoitov wrote: >> On 10/5/15 2:00 PM, Kees Cook wrote: >>> >>> On Mon, Oct 5, 2015 at 1:48 PM, Alexei Starovoitov >>> wrote: >>>> >>>>> In order to let unprivileged users load and execute eBPF programs >>>>> teach verifier to prevent pointer leaks. >>>>> Verifier will prevent >>>>> - any arithmetic on pointers >>>>> (except R10+Imm which is used to compute stack addresses) >>>>> - comparison of pointers >>>>> - passing pointers to helper functions >>>>> - indirectly passing pointers in stack to helper functions >>>>> - returning pointer from bpf program >>>>> - storing pointers into ctx or maps >>> >>> Does the arithmetic restriction include using a pointer as an index to >>> a maps-based tail call? I'm still worried about pointer-based >>> side-effects. >> >> >> the array maps that hold FDs (BPF_MAP_TYPE_PROG_ARRAY and >> BPF_MAP_TYPE_PERF_EVENT_ARRAY) don't have lookup/update accessors >> from the program side, so programs cannot see or manipulate >> those pointers. >> For the former only bpf_tail_call() is allowed that takes integer >> index and jumps to it. And the latter map accessed with >> bpf_perf_event_read() that also takes index only (this helper >> is not available to socket filters anyway). >> Also bpf_tail_call() can only jump to the program of the same type. >> So I'm quite certain it's safe. > > At some point there will be an unprivileged way to create a map, > though, and we don't want to let pointers get poked into the map. yes. exactly. With these two patches non-root can create a map against memlock user limit and have a program store bytes into it (like data from network packet), but it cannot store pointers into it. That's covered by test "unpriv: write pointer into map elem value" I've added new tests for all cases that can 'leak pointer': unpriv: return pointer OK unpriv: add const to pointer OK unpriv: add pointer to pointer OK unpriv: neg pointer OK unpriv: cmp pointer with const OK unpriv: cmp pointer with pointer OK unpriv: pass pointer to printk OK unpriv: pass pointer to helper function OK unpriv: indirectly pass pointer on stack to helper function OK unpriv: mangle pointer on stack 1 OK unpriv: mangle pointer on stack 2 OK unpriv: read pointer from stack in small chunks OK unpriv: write pointer into ctx OK unpriv: write pointer into map elem value OK unpriv: partial copy of pointer OK the most interesting one is 'indirectly pass pointer'. It checks the case where user stores a pointer into a stack and then uses that stack region either as a key for lookup or as part of format string for printk.