mbox series

[bpf-next,00/21] bpf: Sysctl hook

Message ID cover.1553385598.git.rdna@fb.com
Headers show
Series bpf: Sysctl hook | expand

Message

Andrey Ignatov March 24, 2019, 12:12 a.m. UTC 
The patch set introduces new BPF hook for sysctl.

It adds new program type BPF_PROG_TYPE_CGROUP_SYSCTL and attach type
BPF_CGROUP_SYSCTL.

BPF_CGROUP_SYSCTL hook is placed before calling to sysctl's proc_handler so
that accesses (read/write) to sysctl can be controlled for specific cgroup
and either allowed or denied, or traced.

The hook has access to sysctl name, current sysctl value and (on write
only) to new sysctl value via corresponding helpers. New sysctl value can
be overridden by program. Both name and values (current/new) are
represented as strings same way they're visible in /proc/sys/. It is up to
program to parse these strings.

To help with parsing the most common kind of sysctl value, vector of
integers, two new helpers are provided: bpf_strtol and bpf_strtoul with
semantic similar to user space strtol(3) and strtoul(3).

The hook also provides bpf_sysctl context with two fields:
* @write indicates whether sysctl is being read (= 0) or written (= 1);
* @file_pos is sysctl file position to read from or write to, can be
  overridden.

The hook allows to make better isolation for containerized applications
that are run as root so that one container can't change a sysctl and affect
all other containers on a host, make changes to allowed sysctl in a safer
way and simplify sysctl tracing for cgroups.

Patch 1 is preliminary refactoring.
Patch 2 adds new program and attach types.
Patches 3-5 implement helpers to access sysctl name and value.
Patch 6 adds file_pos field to bpf_sysctl context.
Patch 7 updates UAPI in tools.
Patches 8-9 add support for the new hook to libbpf and corresponding test.
Patches 10-14 add selftests for the new hook.
Patch 15 adds support for new arg types to verifier: pointer to integer.
Patch 16 adds bpf_strto{l,ul} helpers to parse integers from sysctl value.
Patch 17 updates UAPI in tools.
Patch 18 updates bpf_helpers.h.
Patch 19 adds selftests for pointer to integer in verifier.
Patches 20-21 add selftests for bpf_strto{l,ul}, including integration
              C based test for sysctl value parsing.


Andrey Ignatov (21):
  bpf: Add base proto function for cgroup-bpf programs
  bpf: Sysctl hook
  bpf: Introduce bpf_sysctl_get_name helper
  bpf: Introduce bpf_sysctl_get_current_value helper
  bpf: Introduce bpf_sysctl_{get,set}_new_value helpers
  bpf: Add file_pos field to bpf_sysctl ctx
  bpf: Sync bpf.h to tools/
  libbpf: Support sysctl hook
  selftests/bpf: Test sysctl section name
  selftests/bpf: Test BPF_CGROUP_SYSCTL
  selftests/bpf: Test bpf_sysctl_get_name helper
  selftests/bpf: Test sysctl_get_current_value helper
  selftests/bpf: Test bpf_sysctl_{get,set}_new_value helpers
  selftests/bpf: Test file_pos field in bpf_sysctl ctx
  bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types
  bpf: Introduce bpf_strtol and bpf_strtoul helpers
  bpf: Sync bpf.h to tools/
  selftests/bpf: Add sysctl and strtoX helpers to bpf_helpers.h
  selftests/bpf: Test ARG_PTR_TO_LONG arg type
  selftests/bpf: Test bpf_strtol and bpf_strtoul helpers
  selftests/bpf: C based test for sysctl and strtoX

 fs/proc/proc_sysctl.c                         |   25 +-
 include/linux/bpf-cgroup.h                    |   21 +
 include/linux/bpf.h                           |    4 +
 include/linux/bpf_types.h                     |    1 +
 include/linux/filter.h                        |   16 +
 include/uapi/linux/bpf.h                      |  139 +-
 kernel/bpf/cgroup.c                           |  364 +++-
 kernel/bpf/helpers.c                          |  131 ++
 kernel/bpf/syscall.c                          |    7 +
 kernel/bpf/verifier.c                         |   30 +
 tools/include/uapi/linux/bpf.h                |  139 +-
 tools/lib/bpf/libbpf.c                        |    3 +
 tools/lib/bpf/libbpf_probes.c                 |    1 +
 tools/testing/selftests/bpf/Makefile          |    3 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   19 +
 .../selftests/bpf/progs/test_sysctl_prog.c    |   85 +
 .../selftests/bpf/test_section_names.c        |    5 +
 tools/testing/selftests/bpf/test_sysctl.c     | 1567 +++++++++++++++++
 .../testing/selftests/bpf/verifier/int_ptr.c  |  160 ++
 19 files changed, 2712 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sysctl_prog.c
 create mode 100644 tools/testing/selftests/bpf/test_sysctl.c
 create mode 100644 tools/testing/selftests/bpf/verifier/int_ptr.c

Comments

Daniel Borkmann March 25, 2019, 10:27 a.m. UTC | #1 
Hi Andrey,

On 03/24/2019 01:12 AM, Andrey Ignatov wrote:
> The patch set introduces new BPF hook for sysctl.
> 
> It adds new program type BPF_PROG_TYPE_CGROUP_SYSCTL and attach type
> BPF_CGROUP_SYSCTL.
> 
> BPF_CGROUP_SYSCTL hook is placed before calling to sysctl's proc_handler so
> that accesses (read/write) to sysctl can be controlled for specific cgroup
> and either allowed or denied, or traced.
> 
> The hook has access to sysctl name, current sysctl value and (on write
> only) to new sysctl value via corresponding helpers. New sysctl value can
> be overridden by program. Both name and values (current/new) are
> represented as strings same way they're visible in /proc/sys/. It is up to
> program to parse these strings.
> 
> To help with parsing the most common kind of sysctl value, vector of
> integers, two new helpers are provided: bpf_strtol and bpf_strtoul with
> semantic similar to user space strtol(3) and strtoul(3).
> 
> The hook also provides bpf_sysctl context with two fields:
> * @write indicates whether sysctl is being read (= 0) or written (= 1);
> * @file_pos is sysctl file position to read from or write to, can be
>   overridden.
> 
> The hook allows to make better isolation for containerized applications
> that are run as root so that one container can't change a sysctl and affect
> all other containers on a host, make changes to allowed sysctl in a safer
> way and simplify sysctl tracing for cgroups.

The change in patch 2 which this whole series is centered around would
need a consent from fs maintainers:

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 4d598a399bbf..72f4a096c146 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -13,6 +13,7 @@
 #include <linux/namei.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/bpf-cgroup.h>
 #include "internal.h"

 static const struct dentry_operations proc_sys_dentry_operations;
@@ -588,6 +589,10 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 	if (!table->proc_handler)
 		goto out;

+	error = BPF_CGROUP_RUN_PROG_SYSCTL(head, table, write);
+	if (error)
+		goto out;
+
 	/* careful: calling conventions are nasty here */
 	res = count;
 	error = table->proc_handler(table, write, buf, &res, ppos);

... but I haven't seen that they are in Cc. Checking MAINTAINERS file,
this would be the following folks:

PROC SYSCTL
M:      Luis Chamberlain <mcgrof@kernel.org>
M:      Kees Cook <keescook@chromium.org>
L:      linux-kernel@vger.kernel.org
L:      linux-fsdevel@vger.kernel.org
S:      Maintained
F:      fs/proc/proc_sysctl.c
F:      include/linux/sysctl.h
F:      kernel/sysctl.c
F:      tools/testing/selftests/sysctl/

Please add them to Cc as well so they can provide Ack, thanks.

> Patch 1 is preliminary refactoring.
> Patch 2 adds new program and attach types.
> Patches 3-5 implement helpers to access sysctl name and value.
> Patch 6 adds file_pos field to bpf_sysctl context.
> Patch 7 updates UAPI in tools.
> Patches 8-9 add support for the new hook to libbpf and corresponding test.
> Patches 10-14 add selftests for the new hook.
> Patch 15 adds support for new arg types to verifier: pointer to integer.
> Patch 16 adds bpf_strto{l,ul} helpers to parse integers from sysctl value.
> Patch 17 updates UAPI in tools.
> Patch 18 updates bpf_helpers.h.
> Patch 19 adds selftests for pointer to integer in verifier.
> Patches 20-21 add selftests for bpf_strto{l,ul}, including integration
>               C based test for sysctl value parsing.
> 
> 
> Andrey Ignatov (21):
>   bpf: Add base proto function for cgroup-bpf programs
>   bpf: Sysctl hook
>   bpf: Introduce bpf_sysctl_get_name helper
>   bpf: Introduce bpf_sysctl_get_current_value helper
>   bpf: Introduce bpf_sysctl_{get,set}_new_value helpers
>   bpf: Add file_pos field to bpf_sysctl ctx
>   bpf: Sync bpf.h to tools/
>   libbpf: Support sysctl hook
>   selftests/bpf: Test sysctl section name
>   selftests/bpf: Test BPF_CGROUP_SYSCTL
>   selftests/bpf: Test bpf_sysctl_get_name helper
>   selftests/bpf: Test sysctl_get_current_value helper
>   selftests/bpf: Test bpf_sysctl_{get,set}_new_value helpers
>   selftests/bpf: Test file_pos field in bpf_sysctl ctx
>   bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types
>   bpf: Introduce bpf_strtol and bpf_strtoul helpers
>   bpf: Sync bpf.h to tools/
>   selftests/bpf: Add sysctl and strtoX helpers to bpf_helpers.h
>   selftests/bpf: Test ARG_PTR_TO_LONG arg type
>   selftests/bpf: Test bpf_strtol and bpf_strtoul helpers
>   selftests/bpf: C based test for sysctl and strtoX
> 
>  fs/proc/proc_sysctl.c                         |   25 +-
>  include/linux/bpf-cgroup.h                    |   21 +
>  include/linux/bpf.h                           |    4 +
>  include/linux/bpf_types.h                     |    1 +
>  include/linux/filter.h                        |   16 +
>  include/uapi/linux/bpf.h                      |  139 +-
>  kernel/bpf/cgroup.c                           |  364 +++-
>  kernel/bpf/helpers.c                          |  131 ++
>  kernel/bpf/syscall.c                          |    7 +
>  kernel/bpf/verifier.c                         |   30 +
>  tools/include/uapi/linux/bpf.h                |  139 +-
>  tools/lib/bpf/libbpf.c                        |    3 +
>  tools/lib/bpf/libbpf_probes.c                 |    1 +
>  tools/testing/selftests/bpf/Makefile          |    3 +-
>  tools/testing/selftests/bpf/bpf_helpers.h     |   19 +
>  .../selftests/bpf/progs/test_sysctl_prog.c    |   85 +
>  .../selftests/bpf/test_section_names.c        |    5 +
>  tools/testing/selftests/bpf/test_sysctl.c     | 1567 +++++++++++++++++
>  .../testing/selftests/bpf/verifier/int_ptr.c  |  160 ++
>  19 files changed, 2712 insertions(+), 8 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_sysctl_prog.c
>  create mode 100644 tools/testing/selftests/bpf/test_sysctl.c
>  create mode 100644 tools/testing/selftests/bpf/verifier/int_ptr.c
>
Andrey Ignatov March 25, 2019, 5:32 p.m. UTC | #2 
Daniel Borkmann <daniel@iogearbox.net> [Mon, 2019-03-25 03:27 -0700]:
> Hi Andrey,

Hi Daniel,

> On 03/24/2019 01:12 AM, Andrey Ignatov wrote:
> > The patch set introduces new BPF hook for sysctl.
...
> The change in patch 2 which this whole series is centered around would
> need a consent from fs maintainers:
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 4d598a399bbf..72f4a096c146 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -13,6 +13,7 @@
>  #include <linux/namei.h>
>  #include <linux/mm.h>
>  #include <linux/module.h>
> +#include <linux/bpf-cgroup.h>
>  #include "internal.h"
> 
>  static const struct dentry_operations proc_sys_dentry_operations;
> @@ -588,6 +589,10 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
>  	if (!table->proc_handler)
>  		goto out;
> 
> +	error = BPF_CGROUP_RUN_PROG_SYSCTL(head, table, write);
> +	if (error)
> +		goto out;
> +
>  	/* careful: calling conventions are nasty here */
>  	res = count;
>  	error = table->proc_handler(table, write, buf, &res, ppos);
> 
> ... but I haven't seen that they are in Cc. Checking MAINTAINERS file,
> this would be the following folks:
> 
> PROC SYSCTL
> M:      Luis Chamberlain <mcgrof@kernel.org>
> M:      Kees Cook <keescook@chromium.org>
> L:      linux-kernel@vger.kernel.org
> L:      linux-fsdevel@vger.kernel.org
> S:      Maintained
> F:      fs/proc/proc_sysctl.c
> F:      include/linux/sysctl.h
> F:      kernel/sysctl.c
> F:      tools/testing/selftests/sysctl/
> 
> Please add them to Cc as well so they can provide Ack, thanks.

Right, I missed that. Will add them as well and send v2. Thanks.