[{"id":1761317,"web_url":"http://patchwork.ozlabs.org/comment/1761317/","msgid":"<1504214241.6901.31.camel@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-08-31T21:17:21","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security\n\tfield to eBPF map","submitter":{"id":466,"url":"http://patchwork.ozlabs.org/api/people/466/","name":"Mimi Zohar","email":"zohar@linux.vnet.ibm.com"},"content":"On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:\n> From: Chenbo Feng <fengc@google.com>\n> \n> Introduce a pointer into struct bpf_map to hold the security information\n> about the map. The actual security struct varies based on the security\n> models implemented. Place the LSM hooks before each of the unrestricted\n> eBPF operations, the map_update_elem and map_delete_elem operations are\n> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n> operations are checked by securtiy_map_read.\n> \n> Signed-off-by: Chenbo Feng <fengc@google.com>\n> ---\n>  include/linux/bpf.h  |  3 +++\n>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 31 insertions(+)\n> \n> diff --git a/include/linux/bpf.h b/include/linux/bpf.h\n> index b69e7a5869ff..ca3e6ff7091d 100644\n> --- a/include/linux/bpf.h\n> +++ b/include/linux/bpf.h\n> @@ -53,6 +53,9 @@ struct bpf_map {\n>  \tstruct work_struct work;\n>  \tatomic_t usercnt;\n>  \tstruct bpf_map *inner_map_meta;\n> +#ifdef CONFIG_SECURITY\n> +\tvoid *security;\n> +#endif\n>  };\n>  \n>  /* function argument constraints */\n> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n> index 045646da97cc..b15580bcf3b1 100644\n> --- a/kernel/bpf/syscall.c\n> +++ b/kernel/bpf/syscall.c\n> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)\n>  \tif (err)\n>  \t\treturn -EINVAL;\n>  \n> +\terr = security_map_create();\n> +\tif (err)\n> +\t\treturn -EACCES;\n\nAny reason not to just return err?\n\nMimi\n\n> +\n>  \t/* find map type and init map: hashtable vs rbtree vs bloom vs ... */\n>  \tmap = find_and_alloc_map(attr);\n>  \tif (IS_ERR(map))\n> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)\n>  \tif (err)\n>  \t\tgoto free_map_nouncharge;\n>  \n> +\terr = security_post_create(map);\n> +\tif (err < 0)\n> +\t\tgoto free_map;\n> +\n>  \terr = bpf_map_alloc_id(map);\n>  \tif (err)\n>  \t\tgoto free_map;\n> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_read(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tkey = memdup_user(ukey, map->key_size);\n>  \tif (IS_ERR(key)) {\n>  \t\terr = PTR_ERR(key);\n> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_modify(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tkey = memdup_user(ukey, map->key_size);\n>  \tif (IS_ERR(key)) {\n>  \t\terr = PTR_ERR(key);\n> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_modify(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tkey = memdup_user(ukey, map->key_size);\n>  \tif (IS_ERR(key)) {\n>  \t\terr = PTR_ERR(key);\n> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_read(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tif (ukey) {\n>  \t\tkey = memdup_user(ukey, map->key_size);\n>  \t\tif (IS_ERR(key)) {\n> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)\n>  \tif (CHECK_ATTR(BPF_PROG_LOAD))\n>  \t\treturn -EINVAL;\n>  \n> +\terr = security_prog_load();\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tif (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)\n>  \t\treturn -EINVAL;\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjwFP2xbmz9t1t\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 07:17:45 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751759AbdHaVRn (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 17:17:43 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:45511 \"EHLO\n\tmx0a-001b2d01.pphosted.com\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751367AbdHaVRl (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 17:17:41 -0400","from pps.filterd (m0098393.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv7VLHPIh129867\n\tfor <netdev@vger.kernel.org>; Thu, 31 Aug 2017 17:17:41 -0400","from e23smtp08.au.ibm.com (e23smtp08.au.ibm.com [202.81.31.141])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cphnumu5r-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor <netdev@vger.kernel.org>; Thu, 31 Aug 2017 17:17:40 -0400","from localhost\n\tby e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor <netdev@vger.kernel.org> from <zohar@linux.vnet.ibm.com>;\n\tFri, 1 Sep 2017 07:17:37 +1000","from d23relay08.au.ibm.com (202.81.31.227)\n\tby e23smtp08.au.ibm.com (202.81.31.205) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tFri, 1 Sep 2017 07:17:36 +1000","from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97])\n\tby d23relay08.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id\n\tv7VLHaxI36372550; Fri, 1 Sep 2017 07:17:36 +1000","from d23av03.au.ibm.com (localhost [127.0.0.1])\n\tby d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id\n\tv7VLHRvA019206; Fri, 1 Sep 2017 07:17:28 +1000","from localhost.localdomain ([9.80.105.29])\n\tby d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id\n\tv7VLHNkS019107; Fri, 1 Sep 2017 07:17:24 +1000"],"Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security\n\tfield to eBPF map","From":"Mimi Zohar <zohar@linux.vnet.ibm.com>","To":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tlinux-security-module@vger.kernel.org","Cc":"Jeffrey Vander Stoep <jeffv@google.com>, netdev@vger.kernel.org,\n\tSELinux <Selinux@tycho.nsa.gov>,\n\tAlexei Starovoitov <alexei.starovoitov@gmail.com>,\n\tlorenzo@google.com, Chenbo Feng <fengc@google.com>","Date":"Thu, 31 Aug 2017 17:17:21 -0400","In-Reply-To":"<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.20.5 (3.20.5-1.fc24) ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","X-TM-AS-MML":"disable","x-cbid":"17083121-0048-0000-0000-0000025A6E09","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17083121-0049-0000-0000-0000480EBE57","Message-Id":"<1504214241.6901.31.camel@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-31_07:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=2\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1708310312","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761349,"web_url":"http://patchwork.ozlabs.org/comment/1761349/","msgid":"<CAMOXUJ=5hyjOs57KPpqPs2NTiL6mP0vega+=GWLb_7sjr_O9fw@mail.gmail.com>","list_archive_url":null,"date":"2017-08-31T22:17:40","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":72270,"url":"http://patchwork.ozlabs.org/api/people/72270/","name":"Chenbo Feng","email":"fengc@google.com"},"content":"On Thu, Aug 31, 2017 at 2:17 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:\n> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:\n>> From: Chenbo Feng <fengc@google.com>\n>>\n>> Introduce a pointer into struct bpf_map to hold the security information\n>> about the map. The actual security struct varies based on the security\n>> models implemented. Place the LSM hooks before each of the unrestricted\n>> eBPF operations, the map_update_elem and map_delete_elem operations are\n>> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n>> operations are checked by securtiy_map_read.\n>>\n>> Signed-off-by: Chenbo Feng <fengc@google.com>\n>> ---\n>>  include/linux/bpf.h  |  3 +++\n>>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++\n>>  2 files changed, 31 insertions(+)\n>>\n>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h\n>> index b69e7a5869ff..ca3e6ff7091d 100644\n>> --- a/include/linux/bpf.h\n>> +++ b/include/linux/bpf.h\n>> @@ -53,6 +53,9 @@ struct bpf_map {\n>>       struct work_struct work;\n>>       atomic_t usercnt;\n>>       struct bpf_map *inner_map_meta;\n>> +#ifdef CONFIG_SECURITY\n>> +     void *security;\n>> +#endif\n>>  };\n>>\n>>  /* function argument constraints */\n>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n>> index 045646da97cc..b15580bcf3b1 100644\n>> --- a/kernel/bpf/syscall.c\n>> +++ b/kernel/bpf/syscall.c\n>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)\n>>       if (err)\n>>               return -EINVAL;\n>>\n>> +     err = security_map_create();\n>> +     if (err)\n>> +             return -EACCES;\n>\n> Any reason not to just return err?\n>\n> Mimi\n>\nNope... return err might be better. I will fix this in next version.\n\nThanks\nChenbo\n>> +\n>>       /* find map type and init map: hashtable vs rbtree vs bloom vs ... */\n>>       map = find_and_alloc_map(attr);\n>>       if (IS_ERR(map))\n>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)\n>>       if (err)\n>>               goto free_map_nouncharge;\n>>\n>> +     err = security_post_create(map);\n>> +     if (err < 0)\n>> +             goto free_map;\n>> +\n>>       err = bpf_map_alloc_id(map);\n>>       if (err)\n>>               goto free_map;\n>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_read(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       key = memdup_user(ukey, map->key_size);\n>>       if (IS_ERR(key)) {\n>>               err = PTR_ERR(key);\n>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_modify(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       key = memdup_user(ukey, map->key_size);\n>>       if (IS_ERR(key)) {\n>>               err = PTR_ERR(key);\n>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_modify(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       key = memdup_user(ukey, map->key_size);\n>>       if (IS_ERR(key)) {\n>>               err = PTR_ERR(key);\n>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_read(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       if (ukey) {\n>>               key = memdup_user(ukey, map->key_size);\n>>               if (IS_ERR(key)) {\n>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)\n>>       if (CHECK_ATTR(BPF_PROG_LOAD))\n>>               return -EINVAL;\n>>\n>> +     err = security_prog_load();\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)\n>>               return -EINVAL;\n>>\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"CvkbBsTA\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjxZn5Ttbz9s81\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 08:17:53 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751345AbdHaWRo (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 18:17:44 -0400","from mail-lf0-f50.google.com ([209.85.215.50]:37850 \"EHLO\n\tmail-lf0-f50.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750925AbdHaWRn (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 18:17:43 -0400","by mail-lf0-f50.google.com with SMTP id y128so3495669lfd.4\n\tfor <netdev@vger.kernel.org>; Thu, 31 Aug 2017 15:17:42 -0700 (PDT)","by 10.46.76.1 with HTTP; Thu, 31 Aug 2017 15:17:40 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=/ufF2BwU2E81Xa7CbxT2FCfRMoMxu0EIh1wLbKwc/Dw=;\n\tb=CvkbBsTAOo9tpfo3TsLdTDkIxXtq7mv0ivFDC/4KWn+vfW0/b9yjM+W2+yug8CMTMS\n\tsJKsH2gAASySHZBpAkmBxsPk8qsIVjD+XZzAtY2b6jurZJTc8W0rX08eUUNf53wkUS1f\n\t7jfmcGhdWq1X2jHc3qjN5n4VJMHC5W4NqzPFpoAno6oezATmJpOZX0GJjc2kF+oEK0r1\n\ttiscblcXS2tTfuUqAfU4SCZPewy5V62QQMHfrVqVWDQetMaKDe/Qe1/UC3XnbL3RlyZk\n\tbzBXPb/MkVwgIAq5CDiAOPjGhfLen/7fo81wx8vJ2QYhuTcLCh6B264OxsxlFWTwiUZX\n\tH1Ag==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=/ufF2BwU2E81Xa7CbxT2FCfRMoMxu0EIh1wLbKwc/Dw=;\n\tb=j8RDNY1DGL2j1ZkMe8BdMxNMGatisJbNhNDGoOqDOW2cuaxJQrL6NHhAbZJKKzng5c\n\t54vhg+qg0LsDMjf46VFs+Xl4gkATEW353LPJlZ6JOxVs3iKjlDIy5bhTcBIbzHap6dbr\n\tStAp4mwfHtWS3DBUSzPPGmJrq5yGdd0gK5NrvwO8mXQi4DB63VhgSs0a1SpmETp0z/is\n\t2ik0Rn6GW/q193iXpkabxWl9iXf7qePrbWNc1R8B3i9uzOR02K3qRiTOguKQRoS/m/6h\n\tC+sRxe2nMgcgNBni2/zXkq5UZf8K6shiyD3GH0PTt3BYVrZq9U6kLx8h9TF/kd4eUL99\n\t68Xg==","X-Gm-Message-State":"AHPjjUghu0OnwIn9qlP83tiT0akqGXuhmeS4MkkKp7cmEtUmnalXvJgB\n\tPqPQxSPghvmgVl8/ZjYgrEVIwQ3rzuP+","X-Google-Smtp-Source":"ADKCNb4lDE/Ox8ahnU6fDYF0BF2uXEuJ7lb6hiUOtRjTyi3au7nx8g4hQZ2eA06epH7M4HIfqfLSYKD7Gs4mwKT4bc0=","X-Received":"by 10.46.81.17 with SMTP id f17mr2870015ljb.73.1504217861038;\n\tThu, 31 Aug 2017 15:17:41 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1504214241.6901.31.camel@linux.vnet.ibm.com>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>\n\t<1504214241.6901.31.camel@linux.vnet.ibm.com>","From":"Chenbo Feng <fengc@google.com>","Date":"Thu, 31 Aug 2017 15:17:40 -0700","Message-ID":"<CAMOXUJ=5hyjOs57KPpqPs2NTiL6mP0vega+=GWLb_7sjr_O9fw@mail.gmail.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","To":"Mimi Zohar <zohar@linux.vnet.ibm.com>","Cc":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tlinux-security-module@vger.kernel.org,\n\tJeffrey Vander Stoep <jeffv@google.com>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tAlexei Starovoitov <alexei.starovoitov@gmail.com>,\n\tLorenzo Colitti <lorenzo@google.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761355,"web_url":"http://patchwork.ozlabs.org/comment/1761355/","msgid":"<59A88FF0.9010605@iogearbox.net>","list_archive_url":null,"date":"2017-08-31T22:38:40","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":65705,"url":"http://patchwork.ozlabs.org/api/people/65705/","name":"Daniel Borkmann","email":"daniel@iogearbox.net"},"content":"On 08/31/2017 10:56 PM, Chenbo Feng wrote:\n> From: Chenbo Feng <fengc@google.com>\n>\n> Introduce a pointer into struct bpf_map to hold the security information\n> about the map. The actual security struct varies based on the security\n> models implemented. Place the LSM hooks before each of the unrestricted\n> eBPF operations, the map_update_elem and map_delete_elem operations are\n> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n> operations are checked by securtiy_map_read.\n>\n> Signed-off-by: Chenbo Feng <fengc@google.com>\n\nAgainst which tree is this by the way, net-next? There are\nchanges here which require a rebase of your set.\n\n> ---\n>   include/linux/bpf.h  |  3 +++\n>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++\n>   2 files changed, 31 insertions(+)\n>\n> diff --git a/include/linux/bpf.h b/include/linux/bpf.h\n> index b69e7a5869ff..ca3e6ff7091d 100644\n> --- a/include/linux/bpf.h\n> +++ b/include/linux/bpf.h\n> @@ -53,6 +53,9 @@ struct bpf_map {\n>   \tstruct work_struct work;\n>   \tatomic_t usercnt;\n>   \tstruct bpf_map *inner_map_meta;\n> +#ifdef CONFIG_SECURITY\n> +\tvoid *security;\n> +#endif\n>   };\n>\n>   /* function argument constraints */\n> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n> index 045646da97cc..b15580bcf3b1 100644\n> --- a/kernel/bpf/syscall.c\n> +++ b/kernel/bpf/syscall.c\n> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)\n>   \tif (err)\n>   \t\treturn -EINVAL;\n>\n> +\terr = security_map_create();\n\nSeems a bit limited to me, don't you want to be able to\nalso differentiate between different map types? Same goes\nfor security_prog_load() wrt prog types, no?\n\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>   \t/* find map type and init map: hashtable vs rbtree vs bloom vs ... */\n>   \tmap = find_and_alloc_map(attr);\n>   \tif (IS_ERR(map))\n> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)\n>   \tif (err)\n>   \t\tgoto free_map_nouncharge;\n>\n> +\terr = security_post_create(map);\n> +\tif (err < 0)\n> +\t\tgoto free_map;\n> +\n\nSo the hook you implement in patch 3/3 does:\n\n+static int selinux_bpf_post_create(struct bpf_map *map)\n+{\n+\tstruct bpf_security_struct *bpfsec;\n+\n+\tbpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);\n+\tif (!bpfsec)\n+\t\treturn -ENOMEM;\n+\n+\tbpfsec->sid = current_sid();\n+\tmap->security = bpfsec;\n+\n+\treturn 0;\n+}\n\nWhere do you kfree() bpfsec when the map gets released\nnormally or in error path?\n\n>   \terr = bpf_map_alloc_id(map);\n>   \tif (err)\n>   \t\tgoto free_map;\n> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>   \tif (IS_ERR(map))\n>   \t\treturn PTR_ERR(map);\n>\n> +\terr = security_map_read(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n\nHow about actually dropping ref?\n\n> +\n>   \tkey = memdup_user(ukey, map->key_size);\n>   \tif (IS_ERR(key)) {\n>   \t\terr = PTR_ERR(key);\n> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>   \tif (IS_ERR(map))\n>   \t\treturn PTR_ERR(map);\n>\n> +\terr = security_map_modify(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n\nDitto ...\n\n>   \tkey = memdup_user(ukey, map->key_size);\n>   \tif (IS_ERR(key)) {\n>   \t\terr = PTR_ERR(key);\n> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)\n>   \tif (IS_ERR(map))\n>   \t\treturn PTR_ERR(map);\n>\n> +\terr = security_map_modify(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n\nDitto ...\n\n>   \tkey = memdup_user(ukey, map->key_size);\n>   \tif (IS_ERR(key)) {\n>   \t\terr = PTR_ERR(key);\n> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)\n>   \tif (IS_ERR(map))\n>   \t\treturn PTR_ERR(map);\n>\n> +\terr = security_map_read(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n\nAnd once again here ... :(\n\n>   \tif (ukey) {\n>   \t\tkey = memdup_user(ukey, map->key_size);\n>   \t\tif (IS_ERR(key)) {\n> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)\n>   \tif (CHECK_ATTR(BPF_PROG_LOAD))\n>   \t\treturn -EINVAL;\n>\n> +\terr = security_prog_load();\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>   \tif (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)\n>   \t\treturn -EINVAL;\n>\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjy2x1k6tz9s7p\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 08:38:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751796AbdHaWiq (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 18:38:46 -0400","from www62.your-server.de ([213.133.104.62]:46189 \"EHLO\n\twww62.your-server.de\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751791AbdHaWio (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 18:38:44 -0400","from [92.105.166.74] (helo=localhost.localdomain)\n\tby www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256)\n\t(Exim 4.85_2) (envelope-from <daniel@iogearbox.net>)\n\tid 1dnY6j-0005Hc-07; Fri, 01 Sep 2017 00:38:41 +0200"],"Message-ID":"<59A88FF0.9010605@iogearbox.net>","Date":"Fri, 01 Sep 2017 00:38:40 +0200","From":"Daniel Borkmann <daniel@iogearbox.net>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.7.0","MIME-Version":"1.0","To":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tlinux-security-module@vger.kernel.org","CC":"Jeffrey Vander Stoep <jeffv@google.com>, netdev@vger.kernel.org,\n\tSELinux <Selinux@tycho.nsa.gov>,\n\tAlexei Starovoitov <alexei.starovoitov@gmail.com>,\n\tlorenzo@google.com, Chenbo Feng <fengc@google.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","In-Reply-To":"<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit","X-Authenticated-Sender":"daniel@iogearbox.net","X-Virus-Scanned":"Clear (ClamAV 0.99.2/23748/Thu Aug 31 22:37:36 2017)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761404,"web_url":"http://patchwork.ozlabs.org/comment/1761404/","msgid":"<CAMOXUJmW7-mOaWq1=qa8FXpvHdXSdAfDSboR-QN-u6DPkkCkkg@mail.gmail.com>","list_archive_url":null,"date":"2017-09-01T00:29:42","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":72270,"url":"http://patchwork.ozlabs.org/api/people/72270/","name":"Chenbo Feng","email":"fengc@google.com"},"content":"On Thu, Aug 31, 2017 at 3:38 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:\n> On 08/31/2017 10:56 PM, Chenbo Feng wrote:\n>>\n>> From: Chenbo Feng <fengc@google.com>\n>>\n>> Introduce a pointer into struct bpf_map to hold the security information\n>> about the map. The actual security struct varies based on the security\n>> models implemented. Place the LSM hooks before each of the unrestricted\n>> eBPF operations, the map_update_elem and map_delete_elem operations are\n>> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n>> operations are checked by securtiy_map_read.\n>>\n>> Signed-off-by: Chenbo Feng <fengc@google.com>\n>\n>\n> Against which tree is this by the way, net-next? There are\n> changes here which require a rebase of your set.\n>\n\nThis patch series is rebased on security subsystem since patch 1/3 is\na patch for\nsecurity system I assume. But I am not sure where this specific patch\nshould go in.\nIf I should submit this one to net-next, I will rebase it against\nnet-next and submit again.\n>> ---\n>>   include/linux/bpf.h  |  3 +++\n>>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++\n>>   2 files changed, 31 insertions(+)\n>>\n>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h\n>> index b69e7a5869ff..ca3e6ff7091d 100644\n>> --- a/include/linux/bpf.h\n>> +++ b/include/linux/bpf.h\n>> @@ -53,6 +53,9 @@ struct bpf_map {\n>>         struct work_struct work;\n>>         atomic_t usercnt;\n>>         struct bpf_map *inner_map_meta;\n>> +#ifdef CONFIG_SECURITY\n>> +       void *security;\n>> +#endif\n>>   };\n>>\n>>   /* function argument constraints */\n>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n>> index 045646da97cc..b15580bcf3b1 100644\n>> --- a/kernel/bpf/syscall.c\n>> +++ b/kernel/bpf/syscall.c\n>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)\n>>         if (err)\n>>                 return -EINVAL;\n>>\n>> +       err = security_map_create();\n>\n>\n> Seems a bit limited to me, don't you want to be able to\n> also differentiate between different map types? Same goes\n> for security_prog_load() wrt prog types, no?\n>\nI don't want to introduce extra complexity into it if not necessary.\nso I only included the\nthing needed for the selinux implementation for now.  But I agree that the map\nand program type information could be useful when people developing more complex\nsecurity checks. I will add a union bpf_attr *attr pointer into the\nsecurity hook functions\nfor future needs.\n>> +       if (err)\n>> +               return -EACCES;\n>> +\n>>         /* find map type and init map: hashtable vs rbtree vs bloom vs ...\n>> */\n>>         map = find_and_alloc_map(attr);\n>>         if (IS_ERR(map))\n>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)\n>>         if (err)\n>>                 goto free_map_nouncharge;\n>>\n>> +       err = security_post_create(map);\n>> +       if (err < 0)\n>> +               goto free_map;\n>> +\n>\n>\n> So the hook you implement in patch 3/3 does:\n>\n> +static int selinux_bpf_post_create(struct bpf_map *map)\n> +{\n> +       struct bpf_security_struct *bpfsec;\n> +\n> +       bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);\n> +       if (!bpfsec)\n> +               return -ENOMEM;\n> +\n> +       bpfsec->sid = current_sid();\n> +       map->security = bpfsec;\n> +\n> +       return 0;\n> +}\n>\n> Where do you kfree() bpfsec when the map gets released\n> normally or in error path?\n>\nWill add it in next version. Thanks for point it out.\n>>         err = bpf_map_alloc_id(map);\n>>         if (err)\n>>                 goto free_map;\n>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>>         if (IS_ERR(map))\n>>                 return PTR_ERR(map);\n>>\n>> +       err = security_map_read(map);\n>> +       if (err)\n>> +               return -EACCES;\n>\n>\n> How about actually dropping ref?\n>\nMay bad, thanks again.\n>> +\n>>         key = memdup_user(ukey, map->key_size);\n>>         if (IS_ERR(key)) {\n>>                 err = PTR_ERR(key);\n>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>>         if (IS_ERR(map))\n>>                 return PTR_ERR(map);\n>>\n>> +       err = security_map_modify(map);\n>> +       if (err)\n>> +               return -EACCES;\n>\n>\n> Ditto ...\n>\n>>         key = memdup_user(ukey, map->key_size);\n>>         if (IS_ERR(key)) {\n>>                 err = PTR_ERR(key);\n>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)\n>>         if (IS_ERR(map))\n>>                 return PTR_ERR(map);\n>>\n>> +       err = security_map_modify(map);\n>> +       if (err)\n>> +               return -EACCES;\n>\n>\n> Ditto ...\n>\n>>         key = memdup_user(ukey, map->key_size);\n>>         if (IS_ERR(key)) {\n>>                 err = PTR_ERR(key);\n>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)\n>>         if (IS_ERR(map))\n>>                 return PTR_ERR(map);\n>>\n>> +       err = security_map_read(map);\n>> +       if (err)\n>> +               return -EACCES;\n>\n>\n> And once again here ... :(\n>\n>\n>>         if (ukey) {\n>>                 key = memdup_user(ukey, map->key_size);\n>>                 if (IS_ERR(key)) {\n>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)\n>>         if (CHECK_ATTR(BPF_PROG_LOAD))\n>>                 return -EINVAL;\n>>\n>> +       err = security_prog_load();\n>> +       if (err)\n>> +               return -EACCES;\n>> +\n>>         if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)\n>>                 return -EINVAL;\n>>\n>>\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"ecefOI6K\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xk0W05GFCz9s7c\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 10:29:48 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751770AbdIAA3q (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 20:29:46 -0400","from mail-lf0-f42.google.com ([209.85.215.42]:37376 \"EHLO\n\tmail-lf0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751404AbdIAA3p (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 20:29:45 -0400","by mail-lf0-f42.google.com with SMTP id y128so4176008lfd.4\n\tfor <netdev@vger.kernel.org>; Thu, 31 Aug 2017 17:29:44 -0700 (PDT)","by 10.46.76.1 with HTTP; Thu, 31 Aug 2017 17:29:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=d0LTr9HACx6xGS2VuvEKZEp8uiWFCsFp+1pyDWnNVaY=;\n\tb=ecefOI6KmlqbSicCjpCRtGPfXIT+15A+PHDX9rxWHO2Btj6aHL/irkpKt3P2l78BPb\n\tbPL6ZbJp+00EzvRC3km9bn+1VddcpFEk9LD9xpNyGEY/TroP2pMFO5DLbs7l1DaPxEPT\n\tmP1d/PYH+2WRXQkT7WEara1Dq9N9flSYUX1Jltx/AtM7ilHn9/OjBCG4RcjgKRgq1AOV\n\tmh2jxcPhsE0RI3en1iM1eztHQi7mwmYxZinE2ib4vP9JldWP/4Svt0JwC6niFiKzXpjP\n\tU5tU2v3nRVwr8NB9dX082zxF3e5YLaahb0PrepXptzzUvCE11SdtRoe1kX+ZdiTAw64K\n\tIGWg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=d0LTr9HACx6xGS2VuvEKZEp8uiWFCsFp+1pyDWnNVaY=;\n\tb=a421hFH0A7TJf80GLVUusasmD0JCoVFbZCM8FV30fy+1B9EXrzZZJ7wa6HCn2qRpbW\n\tmCZZ4GnYwTFRpURNQtKDsp7mF06el6Ofnrkv1Lh6Dy6GA1fh1K6Lu9/VyN5ZWsDDLlhc\n\tvRxV7CzxPA0jXeUpabrC+ue6njcbYs6X96OMj/ED7JJLZK6rZDTlYCVbdMfh4zJ7TrKo\n\ti1ElSDK834FcD6hiosPE0yKTCCW+kt6F0dxnW5Gx6UApt1ujNLWfKTlOIpLVa6wS4ftO\n\tOjbaGf4RGgyYR1svASbnx+Ae02hiKa/dHCDIEbmHcQj3vfe1R2VO/JPsFMqyOH8/1clO\n\tQPkg==","X-Gm-Message-State":"AHPjjUgqev2sLMnzKTwkqFG2btqjW0wzwCwfu1tYLwT4fcc/qE4xumZn\n\trso+4rW9e7In73DO7TJgx1IT5WO4/SX6","X-Google-Smtp-Source":"ADKCNb7B82TUtsv3jYET+bx3hRItMJOCZTKCkBCKEeaR/kijPd7R7zhKdI6O1p/H6/tHTEC1K9vo+JIgxyutatmKXHI=","X-Received":"by 10.25.15.217 with SMTP id 86mr107086lfp.14.1504225783594; Thu,\n\t31 Aug 2017 17:29:43 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<59A88FF0.9010605@iogearbox.net>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>\n\t<59A88FF0.9010605@iogearbox.net>","From":"Chenbo Feng <fengc@google.com>","Date":"Thu, 31 Aug 2017 17:29:42 -0700","Message-ID":"<CAMOXUJmW7-mOaWq1=qa8FXpvHdXSdAfDSboR-QN-u6DPkkCkkg@mail.gmail.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","To":"Daniel Borkmann <daniel@iogearbox.net>","Cc":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tlinux-security-module@vger.kernel.org,\n\tJeffrey Vander Stoep <jeffv@google.com>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tAlexei Starovoitov <alexei.starovoitov@gmail.com>,\n\tLorenzo Colitti <lorenzo@google.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761435,"web_url":"http://patchwork.ozlabs.org/comment/1761435/","msgid":"<20170901020520.uifv6b7tvelgxumf@ast-mbp>","list_archive_url":null,"date":"2017-09-01T02:05:21","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":42586,"url":"http://patchwork.ozlabs.org/api/people/42586/","name":"Alexei Starovoitov","email":"alexei.starovoitov@gmail.com"},"content":"On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:\n> From: Chenbo Feng <fengc@google.com>\n> \n> Introduce a pointer into struct bpf_map to hold the security information\n> about the map. The actual security struct varies based on the security\n> models implemented. Place the LSM hooks before each of the unrestricted\n> eBPF operations, the map_update_elem and map_delete_elem operations are\n> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n> operations are checked by securtiy_map_read.\n> \n> Signed-off-by: Chenbo Feng <fengc@google.com>\n\n...\n\n> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_read(map);\n> +\tif (err)\n> +\t\treturn -EACCES;\n> +\n>  \tkey = memdup_user(ukey, map->key_size);\n>  \tif (IS_ERR(key)) {\n>  \t\terr = PTR_ERR(key);\n> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>  \tif (IS_ERR(map))\n>  \t\treturn PTR_ERR(map);\n>  \n> +\terr = security_map_modify(map);\n\nI don't feel these extra hooks are really thought through.\nWith such hook you'll disallow map_update for given map. That's it.\nThe key/values etc won't be used in such security decision.\nIn such case you don't need such hooks in update/lookup at all.\nOnly in map_creation and object_get calls where FD can be received.\nIn other words I suggest to follow standard unix practices:\nDo permissions checks in open() and allow read/write() if FD is valid.\nSame here. Do permission checks in prog_load/map_create/obj_pin/get\nand that will be enough to jail bpf subsystem.\nbpf cmds that need to be fast (like lookup and update) should not\nhave security hooks.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"g8HR9QwF\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xk2dQ04Gcz9sPm\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 12:05:30 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751439AbdIACF2 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 22:05:28 -0400","from mail-pg0-f65.google.com ([74.125.83.65]:34030 \"EHLO\n\tmail-pg0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750955AbdIACFZ (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 22:05:25 -0400","by mail-pg0-f65.google.com with SMTP id 63so842180pgc.1;\n\tThu, 31 Aug 2017 19:05:25 -0700 (PDT)","from ast-mbp ([2620:10d:c090:180::1:f66])\n\tby smtp.gmail.com with ESMTPSA id\n\t184sm1197681pfg.0.2017.08.31.19.05.23\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 31 Aug 2017 19:05:23 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=vT5SgAH7tTRy2X9HxFy+Xy6+2Ob2albRB3D/0puBzzw=;\n\tb=g8HR9QwFE2jbV/SYJF7APwHsB/umyLTxuVqwwqb2Gx1xbygAlONYgZufRI68+iqLBv\n\tbtyUmVNg0vut2xyVFvxJo4W9C1qbUxof2LP+x0s5n+KIHqzRV9ruMzOV21xy/bf9KNli\n\tmeNVCDPRLmg66fkWVYIOgkkmXgI3mdVuRdgvgmnvFsr+VKT4Igt48U/pKCGDHmmPv6Lc\n\t48bFW6xFaeaJxllmXdfuLr8Xz/d4wA0IWuw7jHyblRtIE74F1KIqaG9wMftmt5/Tlf11\n\tZvNvCjfTwHEfPT190+AqawM9c2I5ciJgUs1SsVZ6jn/Z3/rbqTojdso4jzzacZL95MVm\n\t0LrA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=vT5SgAH7tTRy2X9HxFy+Xy6+2Ob2albRB3D/0puBzzw=;\n\tb=q0hz6Na/rG/NMLRZBIXrxoiWi84TkMlD44YFMvsxz9ujsM1dA1EBzJ0Ph9IIfeusjd\n\tjnFwTtztNhLxWdc2q3yCJ4sGD8YSluYOy4fX0KbVnwIBZH/Sx+dRe2DktEGvw25sQxXV\n\tEFHGNGp8PFtr7EMlFNp0KICXPfgdx4iTVYYiVLFqvSpee5M1XGbKu+EpnxedCkqLrquK\n\trsBTVs5LVRNMc0CH2iR/Hcq5oM/3H6i+RITvY0BTu50IHPf00e6TB5TeSxmOEYWzuz35\n\tBqP+UwiUthBvnfxsOD/LFMxcLp28xVYVbzm6R4XiWjizLbzBDa2oLCRiiV7b6lbxHNNd\n\t+srw==","X-Gm-Message-State":"AHPjjUhSgHjs3/tfBVqt8Odo+UukRFqKUeAbrp6W8PLWgatso+MJryKF\n\tKg+aqzDpOUMUaQ==","X-Google-Smtp-Source":"ADKCNb4fBN6vRbPpYbrZvjtUjhq9a6xb4L4qTI9KmEh5QJ9NBedUTUsWVm35cZnTiC3x92fPparJuQ==","X-Received":"by 10.98.131.73 with SMTP id h70mr511933pfe.327.1504231524972;\n\tThu, 31 Aug 2017 19:05:24 -0700 (PDT)","Date":"Thu, 31 Aug 2017 19:05:21 -0700","From":"Alexei Starovoitov <alexei.starovoitov@gmail.com>","To":"Chenbo Feng <chenbofeng.kernel@gmail.com>","Cc":"Daniel Borkmann <daniel@iogearbox.net>,\n\tlinux-security-module@vger.kernel.org,\n\tJeffrey Vander Stoep <jeffv@google.com>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tlorenzo@google.com, Chenbo Feng <fengc@google.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","Message-ID":"<20170901020520.uifv6b7tvelgxumf@ast-mbp>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170831205635.80256-3-chenbofeng.kernel@gmail.com>","User-Agent":"NeoMutt/20170421 (1.8.2)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761476,"web_url":"http://patchwork.ozlabs.org/comment/1761476/","msgid":"<CABXk95AaepfP94aPZM6kOBgKVM8e2Ma8g63DLdpOLAAyDF=eSQ@mail.gmail.com>","list_archive_url":null,"date":"2017-09-01T05:50:45","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":72274,"url":"http://patchwork.ozlabs.org/api/people/72274/","name":"Jeff Vander Stoep","email":"jeffv@google.com"},"content":"On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov\n<alexei.starovoitov@gmail.com> wrote:\n> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:\n>> From: Chenbo Feng <fengc@google.com>\n>>\n>> Introduce a pointer into struct bpf_map to hold the security information\n>> about the map. The actual security struct varies based on the security\n>> models implemented. Place the LSM hooks before each of the unrestricted\n>> eBPF operations, the map_update_elem and map_delete_elem operations are\n>> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n>> operations are checked by securtiy_map_read.\n>>\n>> Signed-off-by: Chenbo Feng <fengc@google.com>\n>\n> ...\n>\n>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_read(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       key = memdup_user(ukey, map->key_size);\n>>       if (IS_ERR(key)) {\n>>               err = PTR_ERR(key);\n>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_modify(map);\n>\n> I don't feel these extra hooks are really thought through.\n> With such hook you'll disallow map_update for given map. That's it.\n> The key/values etc won't be used in such security decision.\n> In such case you don't need such hooks in update/lookup at all.\n> Only in map_creation and object_get calls where FD can be received.\n> In other words I suggest to follow standard unix practices:\n> Do permissions checks in open() and allow read/write() if FD is valid.\n> Same here. Do permission checks in prog_load/map_create/obj_pin/get\n> and that will be enough to jail bpf subsystem.\n> bpf cmds that need to be fast (like lookup and update) should not\n> have security hooks.\n>\n\nI do think we want to distinguish between read/write (or read/modify)\nfor these objects. Essentially, we want to implement the example\ndescribed in patch 0/3 where eBPF objects can be passed to less\nprivileged processes which can read, but not modify the map. What\nwould be the best way to do this? Add a mode field to the bpf_map\nstruct?","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"A+hgutoL\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xk7dQ2vDKz9s8J\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  1 Sep 2017 15:50:50 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751385AbdIAFus (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 1 Sep 2017 01:50:48 -0400","from mail-qk0-f174.google.com ([209.85.220.174]:38095 \"EHLO\n\tmail-qk0-f174.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751359AbdIAFuq (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 1 Sep 2017 01:50:46 -0400","by mail-qk0-f174.google.com with SMTP id a21so6664514qkg.5\n\tfor <netdev@vger.kernel.org>; Thu, 31 Aug 2017 22:50:46 -0700 (PDT)","by 10.200.53.122 with HTTP; Thu, 31 Aug 2017 22:50:45 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=RAC9Ri4TzfN6104G+icoSgMHBhngxbckDi4J3YbhtYM=;\n\tb=A+hgutoLsoTOBo/M6vd+rKRHTIusySo1ZBxMiXnlTBmqn/j8HN2mu7iWzogTtuU+zE\n\tpyumhHAZz2NoY5il/t9vtC04J7LlsSjTQ9jliuwzKZiLaI/nJFfgXCG+CAtWZBxN3fcg\n\tcu5hjYfcZb6PO1rOV+nRwg6hq7TijcAKWhjC/OrX/YrTUyFbUY7sfruuCeM8sF5PXp+a\n\t9aPmRQg9gpC5sSOl6QvdyQU6zrD897Ilvg66wCmshIScFeuMlDpbnnJgHDkuh1urUZhb\n\t8mNKUcZxxIQz2KY5nf3zdC4zH6LGVcqzVVPRGMQ76PX55zCtKUxreRM+PB65JBkNa0Re\n\tWvUw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=RAC9Ri4TzfN6104G+icoSgMHBhngxbckDi4J3YbhtYM=;\n\tb=Bcrs+w2ZXFfJ1FJuB7NS0t8i+rrdLKgD9EXwxYEpPQikWf5kXyHyctOKVuanqfcKoC\n\tiNiZoW1QehW+dNwUDxqu2iwyEydanYLbQ3j519VLJbbtjDzoLuWHUWmZWBYBH5tbdr5N\n\t+Z1YFCV6cRwZX/xsCQ7UMhIeT7GaTK6Fb33xsYNv2VrJa/TMzUWApB/AIuK+BS5NHRhz\n\t4gfnf0oaedE/mrXB2UdRZ/slxJfWR/eSxzdkrgVOk0HpS9ED8n/VyRWUv6s/gMa4Lqob\n\tmEWRVO31H4Q7ny8FclXewdMOq6DSwST3A47mIZhYVdv0xSu/V2LmJeDQWNekDpQ85gIu\n\tz6Ng==","X-Gm-Message-State":"AHPjjUg/Z2xFy74k40OStd2lOaDDFVlFCsiDc/+FAiG1ZFIaJxkSe7/f\n\taqqKMn7Q0FuJFvcvWoLLylW8hhOUW2C/","X-Google-Smtp-Source":"ADKCNb7Xrs9AoWTJt2LDMaJydSVsjIEGCTY1UMC2v3mPKeiHJN3+EO8tymF3MlfTpBYmxXXMcdp7P74Nk7QaqH/wn4Q=","X-Received":"by 10.55.71.206 with SMTP id u197mr1135835qka.339.1504245046099; \n\tThu, 31 Aug 2017 22:50:46 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170901020520.uifv6b7tvelgxumf@ast-mbp>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>\n\t<20170901020520.uifv6b7tvelgxumf@ast-mbp>","From":"Jeffrey Vander Stoep <jeffv@google.com>","Date":"Thu, 31 Aug 2017 22:50:45 -0700","Message-ID":"<CABXk95AaepfP94aPZM6kOBgKVM8e2Ma8g63DLdpOLAAyDF=eSQ@mail.gmail.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","To":"Alexei Starovoitov <alexei.starovoitov@gmail.com>","Cc":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tDaniel Borkmann <daniel@iogearbox.net>,\n\tLSM List <linux-security-module@vger.kernel.org>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tLorenzo Colitti <lorenzo@google.com>, Chenbo Feng <fengc@google.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1763691,"web_url":"http://patchwork.ozlabs.org/comment/1763691/","msgid":"<CAMOXUJnF=UoWM7xm-Uk-zyhA1N9exjtDf8Xt+_EF2Kj6YUV0OQ@mail.gmail.com>","list_archive_url":null,"date":"2017-09-05T21:59:38","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":72270,"url":"http://patchwork.ozlabs.org/api/people/72270/","name":"Chenbo Feng","email":"fengc@google.com"},"content":"On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov\n<alexei.starovoitov@gmail.com> wrote:\n> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:\n>> From: Chenbo Feng <fengc@google.com>\n>>\n>> Introduce a pointer into struct bpf_map to hold the security information\n>> about the map. The actual security struct varies based on the security\n>> models implemented. Place the LSM hooks before each of the unrestricted\n>> eBPF operations, the map_update_elem and map_delete_elem operations are\n>> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n>> operations are checked by securtiy_map_read.\n>>\n>> Signed-off-by: Chenbo Feng <fengc@google.com>\n>\n> ...\n>\n>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_read(map);\n>> +     if (err)\n>> +             return -EACCES;\n>> +\n>>       key = memdup_user(ukey, map->key_size);\n>>       if (IS_ERR(key)) {\n>>               err = PTR_ERR(key);\n>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n>>       if (IS_ERR(map))\n>>               return PTR_ERR(map);\n>>\n>> +     err = security_map_modify(map);\n>\n> I don't feel these extra hooks are really thought through.\n> With such hook you'll disallow map_update for given map. That's it.\n> The key/values etc won't be used in such security decision.\n> In such case you don't need such hooks in update/lookup at all.\n> Only in map_creation and object_get calls where FD can be received.\n> In other words I suggest to follow standard unix practices:\n> Do permissions checks in open() and allow read/write() if FD is valid.\n> Same here. Do permission checks in prog_load/map_create/obj_pin/get\n> and that will be enough to jail bpf subsystem.\n> bpf cmds that need to be fast (like lookup and update) should not\n> have security hooks.\n>\nThanks for pointing out this. I agree we should only do checks on\ncreating and passing\nthe object from one processes to another. And if we can still\ndistinguish the read/write operation\nwhen obtaining the file fd, that would be great. But that may require\nus to add a new mode\nfield into bpf_map struct and change the attribute struct when doing\nthe bpf syscall. How do you\nthink about this approach? Or we can do simple checks for\nbpf_obj_create and bpf_obj_use when\ncreating the object and passing the object respectively but this\nsolution cannot distinguish map modify and\nread.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"v6LZ0Ikb\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xn0y70xY8z9s83\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  6 Sep 2017 08:00:15 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754181AbdIEV7u (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 5 Sep 2017 17:59:50 -0400","from mail-lf0-f53.google.com ([209.85.215.53]:37085 \"EHLO\n\tmail-lf0-f53.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754159AbdIEV7l (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Tue, 5 Sep 2017 17:59:41 -0400","by mail-lf0-f53.google.com with SMTP id 80so9026289lfy.4\n\tfor <netdev@vger.kernel.org>; Tue, 05 Sep 2017 14:59:40 -0700 (PDT)","by 10.46.76.1 with HTTP; Tue, 5 Sep 2017 14:59:38 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=dEGASX8hP1CU8K4Erps7msFKpY0S2lYfco3TSRc0Ong=;\n\tb=v6LZ0IkbqiY/t0CM0S5/zA4wJsAy5mv9fDlf6KbW4PYUblaSPE2CJ/jaizjuLVwqx3\n\tjbSa0k/3qiFg6R+HKs6ZFwH28pa7nsaooOfVRTOCjf2qPu/lqeBHaX2ULY8jgLKgs5Yi\n\tv2appoV3z3xC5mDb1q5DzlioNjTqTy6lgzRN433WIR72VvwqjGHIxaVhb3xi/f4BiXol\n\tZfhwZ0ve4qaSm90GMcVhy8eFPy/90F3ioL0ddupRT6XcY/PilujQeRkIc7/+ENne2oNb\n\tvmjxWNbvs83myM13hF+kO+6xK+RPKtzVBALVsvptKLs7CmZZz2rhaof2EQnrdD9al6dt\n\tNxKg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=dEGASX8hP1CU8K4Erps7msFKpY0S2lYfco3TSRc0Ong=;\n\tb=ghbwpUhO7k4uQYXphpedstPu9Ao3FmYx0dy5Qp0sNvHI3QohbnTuUrr6lcOi9dPaEY\n\tqjabybI/fGpItfDbTK4ZYM0dHlWDGToTZh4rqtVrfjVakMtBudJ5x4s9ycTGyWOzi+bs\n\tC3BBotG0pQE6IujGh8kVNGR1QN9gtxHMtra3gnjWeZQhxX8YMFtDnoMZO4zdGezNSpWQ\n\tjPHRCiALPVXRTlB1y7AChX+eG0bkgNmZNpRXSOSx3ZRuYovWIBh5ftHE2EE1PKxFSqwd\n\tTJKpS1Ge1+aZpyPgzgv8Hete/WAlr+WYVJA0dkWMfrFxqLu0DbmNQTzaeL3xYyfPOT6S\n\tMg2w==","X-Gm-Message-State":"AHPjjUhQF2IMkFlhBcif8ZMXxAw8UyRjnIv3R982cKnXuOvhLwtByBZx\n\t31VBckxgIOWT6kims2iwKao/o72pWZft","X-Google-Smtp-Source":"ADKCNb4aEp0F9ZpLmzRwAoAEyP4IwFVZjNwG7jMz86yuBHJ0f3MNaxKMSNxJL8Rq4J1v2AN4/YE77eVu8IBToExKuC8=","X-Received":"by 10.46.86.18 with SMTP id k18mr177759ljb.73.1504648779225; Tue,\n\t05 Sep 2017 14:59:39 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170901020520.uifv6b7tvelgxumf@ast-mbp>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>\n\t<20170901020520.uifv6b7tvelgxumf@ast-mbp>","From":"Chenbo Feng <fengc@google.com>","Date":"Tue, 5 Sep 2017 14:59:38 -0700","Message-ID":"<CAMOXUJnF=UoWM7xm-Uk-zyhA1N9exjtDf8Xt+_EF2Kj6YUV0OQ@mail.gmail.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","To":"Alexei Starovoitov <alexei.starovoitov@gmail.com>","Cc":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tDaniel Borkmann <daniel@iogearbox.net>,\n\tlinux-security-module@vger.kernel.org,\n\tJeffrey Vander Stoep <jeffv@google.com>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tLorenzo Colitti <lorenzo@google.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1763747,"web_url":"http://patchwork.ozlabs.org/comment/1763747/","msgid":"<20170906003913.h6khoama7db4rw6e@ast-mbp>","list_archive_url":null,"date":"2017-09-06T00:39:14","subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","submitter":{"id":42586,"url":"http://patchwork.ozlabs.org/api/people/42586/","name":"Alexei Starovoitov","email":"alexei.starovoitov@gmail.com"},"content":"On Tue, Sep 05, 2017 at 02:59:38PM -0700, Chenbo Feng wrote:\n> On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov\n> <alexei.starovoitov@gmail.com> wrote:\n> > On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:\n> >> From: Chenbo Feng <fengc@google.com>\n> >>\n> >> Introduce a pointer into struct bpf_map to hold the security information\n> >> about the map. The actual security struct varies based on the security\n> >> models implemented. Place the LSM hooks before each of the unrestricted\n> >> eBPF operations, the map_update_elem and map_delete_elem operations are\n> >> checked by security_map_modify. The map_lookup_elem and map_get_next_key\n> >> operations are checked by securtiy_map_read.\n> >>\n> >> Signed-off-by: Chenbo Feng <fengc@google.com>\n> >\n> > ...\n> >\n> >> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)\n> >>       if (IS_ERR(map))\n> >>               return PTR_ERR(map);\n> >>\n> >> +     err = security_map_read(map);\n> >> +     if (err)\n> >> +             return -EACCES;\n> >> +\n> >>       key = memdup_user(ukey, map->key_size);\n> >>       if (IS_ERR(key)) {\n> >>               err = PTR_ERR(key);\n> >> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)\n> >>       if (IS_ERR(map))\n> >>               return PTR_ERR(map);\n> >>\n> >> +     err = security_map_modify(map);\n> >\n> > I don't feel these extra hooks are really thought through.\n> > With such hook you'll disallow map_update for given map. That's it.\n> > The key/values etc won't be used in such security decision.\n> > In such case you don't need such hooks in update/lookup at all.\n> > Only in map_creation and object_get calls where FD can be received.\n> > In other words I suggest to follow standard unix practices:\n> > Do permissions checks in open() and allow read/write() if FD is valid.\n> > Same here. Do permission checks in prog_load/map_create/obj_pin/get\n> > and that will be enough to jail bpf subsystem.\n> > bpf cmds that need to be fast (like lookup and update) should not\n> > have security hooks.\n> >\n> Thanks for pointing out this. I agree we should only do checks on\n> creating and passing\n> the object from one processes to another. And if we can still\n> distinguish the read/write operation\n> when obtaining the file fd, that would be great. But that may require\n> us to add a new mode\n> field into bpf_map struct and change the attribute struct when doing\n> the bpf syscall. How do you\n> think about this approach? Or we can do simple checks for\n> bpf_obj_create and bpf_obj_use when\n> creating the object and passing the object respectively but this\n> solution cannot distinguish map modify and\n> read.\n\niirc the idea to have read only maps was brought up in the past\n(unfortunately no one took a stab at implementing it), but imo\nthat's better then relying on lsm to provide such restriction\nand more secure, since even if you disable map_update via lsm,\nthe user can craft a program just to udpate the map from it.\nFor bpffs we already test for inode_permission(inode, MAY_WRITE);\nduring BPF_OBJ_GET command and we can extend this facility further.\nAlso we can allow dropping 'write' permissions from the map\n(internally implemented via flag in struct bpf_map), so\nupdate/delete operations won't be allowed.\nThis flag will be checked by syscall during map_update/delete\nand by the verifier if such read-only map is used by the program\nbeing loaded, so map_update/helpers won't be allowed in\nsuch program.\nWould also be good to support read-only programs as well\n(progs that are read-only from kernel state point of view)\nThey won't be able to modify packets, maps, etc.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"aS1dzIm6\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xn4Tp4mS2z9t2y\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  6 Sep 2017 10:39:26 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752471AbdIFAjV (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 5 Sep 2017 20:39:21 -0400","from mail-pg0-f66.google.com ([74.125.83.66]:36674 \"EHLO\n\tmail-pg0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752194AbdIFAjS (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Tue, 5 Sep 2017 20:39:18 -0400","by mail-pg0-f66.google.com with SMTP id d8so2728701pgt.3;\n\tTue, 05 Sep 2017 17:39:18 -0700 (PDT)","from ast-mbp ([2620:10d:c090:180::1:ca20])\n\tby smtp.gmail.com with ESMTPSA id\n\ts8sm167246pgf.78.2017.09.05.17.39.16\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tTue, 05 Sep 2017 17:39:17 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=MVN2rPRJNDUlL6dxpUwVQJW0+pwM1uYKUie5Mpt/cDs=;\n\tb=aS1dzIm6/NhqH3fAY67pIAp1BT3lJZ1jBvTdZesKQDM0b9uOna8CON7G2K8ZvW/kf+\n\tYyi5pk/BzC8AggLNSniLNPeFreR0rjpXanmcQQ0jWORQeTAsLmOS5pOXjSk1PAfTxCGT\n\tTKYB4Mqpx5zDRcPTGAltfmR1bcsxzK8J0EQfCx5rCsnm9q1aWEUodj9r8fPc3TwhywKb\n\tPfG0pvc02XYb1/GEN3s9GprD4XXTow7rUuxZG3oIC+5sduv2OA5A0u7MOyr9M54T+JRJ\n\t8PWIY04aeooJ5E27RzJ3g7Oe3ZlG8yxr9UDo+iHOdHratUBlO1cbgV7Ks7yNYlq+4snB\n\tdB+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=MVN2rPRJNDUlL6dxpUwVQJW0+pwM1uYKUie5Mpt/cDs=;\n\tb=OakzNbQ7SOgUhotpFKw1xMFdXMHwaF61EHSZzf6O04zS+WhtaTjXZT5AOhHEcsIXUa\n\t8QOMp5jf5uYynFILPI1vrReGtwZF701iRwNpuzDoXrNxa0sA1auYKof0H+cRp6f+Pf8X\n\tK25ecs6BqSilKDC7Yk10v/a4Qk7H4K9a5bZJK1Te3ZHiHt+ZwWN/G0Ev/AGM1C4DaGWH\n\teBLbfoaBLYlSokUVpbkuuMd1Yxneir2CaSAieXa0Lb9QKf4P5p8oL5BM+KSOjF7U8NS9\n\tJNeCr1q8Ve9eEmXSHXKePTNML5hZQqOH9I4sINb/NOZaw71GM99c/FPqa2z017ofzX7H\n\tfIlA==","X-Gm-Message-State":"AHPjjUj5SX4otGKl6qNpsGTJgwQoRQIGias35wqHwAMBD3APt/GYa4Hl\n\tPKrNcZdtKxZHoA==","X-Google-Smtp-Source":"ADKCNb6wH+bhJ/Qd0jsAzRxEZMCtuzmCMAAxErRuYSp+zlLsI42Wuhtr7pJmlHNyI8XlGPlqdidJYg==","X-Received":"by 10.99.55.92 with SMTP id g28mr5867546pgn.59.1504658358306;\n\tTue, 05 Sep 2017 17:39:18 -0700 (PDT)","Date":"Tue, 5 Sep 2017 17:39:14 -0700","From":"Alexei Starovoitov <alexei.starovoitov@gmail.com>","To":"Chenbo Feng <fengc@google.com>","Cc":"Chenbo Feng <chenbofeng.kernel@gmail.com>,\n\tDaniel Borkmann <daniel@iogearbox.net>,\n\tlinux-security-module@vger.kernel.org,\n\tJeffrey Vander Stoep <jeffv@google.com>,\n\tnetdev@vger.kernel.org, SELinux <Selinux@tycho.nsa.gov>,\n\tLorenzo Colitti <lorenzo@google.com>","Subject":"Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field\n\tto eBPF map","Message-ID":"<20170906003913.h6khoama7db4rw6e@ast-mbp>","References":"<20170831205635.80256-1-chenbofeng.kernel@gmail.com>\n\t<20170831205635.80256-3-chenbofeng.kernel@gmail.com>\n\t<20170901020520.uifv6b7tvelgxumf@ast-mbp>\n\t<CAMOXUJnF=UoWM7xm-Uk-zyhA1N9exjtDf8Xt+_EF2Kj6YUV0OQ@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<CAMOXUJnF=UoWM7xm-Uk-zyhA1N9exjtDf8Xt+_EF2Kj6YUV0OQ@mail.gmail.com>","User-Agent":"NeoMutt/20170421 (1.8.2)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]