[bpf-next,v1,09/13] bpf: lsm: Add a helper function bpf_lsm_event_output
diff mbox series

Message ID 20191220154208.15895-10-kpsingh@chromium.org
State New
Headers show
Series
  • MAC and Audit policy using eBPF (KRSI)
Related show

Commit Message

KP Singh Dec. 20, 2019, 3:42 p.m. UTC
From: KP Singh <kpsingh@google.com>

This helper is similar to bpf_perf_event_output except that
it does need a ctx argument which is more usable in the
BTF based LSM programs where the context is converted to
the signature of the attacthed BTF type.

An example usage of this function would be:

struct {
         __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
         __uint(key_size, sizeof(int));
         __uint(value_size, sizeof(u32));
} perf_map SEC(".maps");

BPF_TRACE_1(bpf_prog1, "lsm/bprm_check_security,
	    struct linux_binprm *, bprm)
{
	char buf[BUF_SIZE];
	int len;
	u64 flags = BPF_F_CURRENT_CPU;

	/* some logic that fills up buf with len data */
	len = fill_up_buf(buf);
	if (len < 0)
		return len;
	if (len > BU)
		return 0;

	bpf_lsm_event_output(&perf_map, flags, buf, len);
	return 0;
}

Signed-off-by: KP Singh <kpsingh@google.com>
---
 include/uapi/linux/bpf.h       | 10 +++++++++-
 kernel/bpf/verifier.c          |  1 +
 security/bpf/ops.c             | 21 +++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 10 +++++++++-
 4 files changed, 40 insertions(+), 2 deletions(-)

Comments

Andrii Nakryiko Dec. 24, 2019, 6:36 a.m. UTC | #1
On Fri, Dec 20, 2019 at 7:43 AM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> This helper is similar to bpf_perf_event_output except that
> it does need a ctx argument which is more usable in the
> BTF based LSM programs where the context is converted to
> the signature of the attacthed BTF type.
>
> An example usage of this function would be:
>
> struct {
>          __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
>          __uint(key_size, sizeof(int));
>          __uint(value_size, sizeof(u32));
> } perf_map SEC(".maps");
>
> BPF_TRACE_1(bpf_prog1, "lsm/bprm_check_security,
>             struct linux_binprm *, bprm)
> {
>         char buf[BUF_SIZE];
>         int len;
>         u64 flags = BPF_F_CURRENT_CPU;
>
>         /* some logic that fills up buf with len data */
>         len = fill_up_buf(buf);
>         if (len < 0)
>                 return len;
>         if (len > BU)
>                 return 0;
>
>         bpf_lsm_event_output(&perf_map, flags, buf, len);

This seems to be generally useful and not LSM-specific, so maybe name
it more generically as bpf_event_output instead?

I'm also curious why we needed both bpf_perf_event_output and
bpf_perf_event_output_raw_tp, if it could be done as simply as you did
it here. What's different between those three and why your
bpf_lsm_event_output doesn't need pt_regs passed into them?

>         return 0;
> }
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>  include/uapi/linux/bpf.h       | 10 +++++++++-
>  kernel/bpf/verifier.c          |  1 +
>  security/bpf/ops.c             | 21 +++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 10 +++++++++-
>  4 files changed, 40 insertions(+), 2 deletions(-)
>

[...]
KP Singh Dec. 30, 2019, 3:11 p.m. UTC | #2
On 23-Dec 22:36, Andrii Nakryiko wrote:
> On Fri, Dec 20, 2019 at 7:43 AM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >
> > This helper is similar to bpf_perf_event_output except that
> > it does need a ctx argument which is more usable in the
> > BTF based LSM programs where the context is converted to
> > the signature of the attacthed BTF type.
> >
> > An example usage of this function would be:
> >
> > struct {
> >          __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> >          __uint(key_size, sizeof(int));
> >          __uint(value_size, sizeof(u32));
> > } perf_map SEC(".maps");
> >
> > BPF_TRACE_1(bpf_prog1, "lsm/bprm_check_security,
> >             struct linux_binprm *, bprm)
> > {
> >         char buf[BUF_SIZE];
> >         int len;
> >         u64 flags = BPF_F_CURRENT_CPU;
> >
> >         /* some logic that fills up buf with len data */
> >         len = fill_up_buf(buf);
> >         if (len < 0)
> >                 return len;
> >         if (len > BU)
> >                 return 0;
> >
> >         bpf_lsm_event_output(&perf_map, flags, buf, len);
> 
> This seems to be generally useful and not LSM-specific, so maybe name
> it more generically as bpf_event_output instead?

Agreed, I am happy to rename this.

> 
> I'm also curious why we needed both bpf_perf_event_output and
> bpf_perf_event_output_raw_tp, if it could be done as simply as you did
> it here. What's different between those three and why your
> bpf_lsm_event_output doesn't need pt_regs passed into them?

That's because my implementation uses the following function from
bpf_trace.c:

u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy)

This does not require a pt_regs argument and handles fetching them
internally:

	regs = this_cpu_ptr(&bpf_pt_regs.regs[nest_level - 1]);

	perf_fetch_caller_regs(regs);
	perf_sample_data_init(sd, 0, 0);
	sd->raw = &raw;

	ret = __bpf_perf_event_output(regs, map, flags, sd);

- KP

> 
> >         return 0;
> > }
> >
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > ---
> >  include/uapi/linux/bpf.h       | 10 +++++++++-
> >  kernel/bpf/verifier.c          |  1 +
> >  security/bpf/ops.c             | 21 +++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h | 10 +++++++++-
> >  4 files changed, 40 insertions(+), 2 deletions(-)
> >
> 
> [...]
Andrii Nakryiko Dec. 30, 2019, 6:56 p.m. UTC | #3
On Mon, Dec 30, 2019 at 7:11 AM KP Singh <kpsingh@chromium.org> wrote:
>
> On 23-Dec 22:36, Andrii Nakryiko wrote:
> > On Fri, Dec 20, 2019 at 7:43 AM KP Singh <kpsingh@chromium.org> wrote:
> > >
> > > From: KP Singh <kpsingh@google.com>
> > >
> > > This helper is similar to bpf_perf_event_output except that
> > > it does need a ctx argument which is more usable in the
> > > BTF based LSM programs where the context is converted to
> > > the signature of the attacthed BTF type.
> > >
> > > An example usage of this function would be:
> > >
> > > struct {
> > >          __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> > >          __uint(key_size, sizeof(int));
> > >          __uint(value_size, sizeof(u32));
> > > } perf_map SEC(".maps");
> > >
> > > BPF_TRACE_1(bpf_prog1, "lsm/bprm_check_security,
> > >             struct linux_binprm *, bprm)
> > > {
> > >         char buf[BUF_SIZE];
> > >         int len;
> > >         u64 flags = BPF_F_CURRENT_CPU;
> > >
> > >         /* some logic that fills up buf with len data */
> > >         len = fill_up_buf(buf);
> > >         if (len < 0)
> > >                 return len;
> > >         if (len > BU)
> > >                 return 0;
> > >
> > >         bpf_lsm_event_output(&perf_map, flags, buf, len);
> >
> > This seems to be generally useful and not LSM-specific, so maybe name
> > it more generically as bpf_event_output instead?
>
> Agreed, I am happy to rename this.
>
> >
> > I'm also curious why we needed both bpf_perf_event_output and
> > bpf_perf_event_output_raw_tp, if it could be done as simply as you did
> > it here. What's different between those three and why your
> > bpf_lsm_event_output doesn't need pt_regs passed into them?
>
> That's because my implementation uses the following function from
> bpf_trace.c:
>
> u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>                      void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy)
>
> This does not require a pt_regs argument and handles fetching them
> internally:
>
>         regs = this_cpu_ptr(&bpf_pt_regs.regs[nest_level - 1]);
>
>         perf_fetch_caller_regs(regs);
>         perf_sample_data_init(sd, 0, 0);
>         sd->raw = &raw;
>
>         ret = __bpf_perf_event_output(regs, map, flags, sd);
>
> - KP

Yeah, I saw that bit. I guess I'm confused why we couldn't do the same
for, say, raw_tracepoint case. Now Jiri Olsa is adding another similar
helper doing its own storage of pt_regs. If all of them can share the
same (even if with bigger nest_level) array of pt_regs, that would
great.

>
> >
> > >         return 0;
> > > }
> > >
> > > Signed-off-by: KP Singh <kpsingh@google.com>
> > > ---
> > >  include/uapi/linux/bpf.h       | 10 +++++++++-
> > >  kernel/bpf/verifier.c          |  1 +
> > >  security/bpf/ops.c             | 21 +++++++++++++++++++++
> > >  tools/include/uapi/linux/bpf.h | 10 +++++++++-
> > >  4 files changed, 40 insertions(+), 2 deletions(-)
> > >
> >
> > [...]

Patch
diff mbox series

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index fc64ae865526..3511fa271c9b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2823,6 +2823,13 @@  union bpf_attr {
  * 	Return
  * 		On success, the strictly positive length of the string,	including
  * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_lsm_event_output(struct bpf_map *map, u64 flags, void *data, u64 size)
+ *	Description
+ *		This helper is similar to bpf_perf_event_output except that it
+ *		it does not need a context argument.
+ *	Return
+ *		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2940,7 +2947,8 @@  union bpf_attr {
 	FN(probe_read_user),		\
 	FN(probe_read_kernel),		\
 	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(lsm_event_output),		\
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0d1231d9c1ef..ff050fd71e9f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3641,6 +3641,7 @@  static int check_map_func_compatibility(struct bpf_verifier_env *env,
 		if (func_id != BPF_FUNC_perf_event_read &&
 		    func_id != BPF_FUNC_perf_event_output &&
 		    func_id != BPF_FUNC_skb_output &&
+		    func_id != BPF_FUNC_lsm_event_output &&
 		    func_id != BPF_FUNC_perf_event_read_value)
 			goto error;
 		break;
diff --git a/security/bpf/ops.c b/security/bpf/ops.c
index eb8a8db28109..e9aae2ce718c 100644
--- a/security/bpf/ops.c
+++ b/security/bpf/ops.c
@@ -129,6 +129,25 @@  int bpf_lsm_detach(const union bpf_attr *attr)
 const struct bpf_prog_ops lsm_prog_ops = {
 };
 
+BPF_CALL_4(bpf_lsm_event_output,
+	   struct bpf_map *, map, u64, flags, void *, data, u64, size)
+{
+	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
+		return -EINVAL;
+
+	return bpf_event_output(map, flags, data, size, NULL, 0, NULL);
+}
+
+static const struct bpf_func_proto bpf_lsm_event_output_proto =  {
+	.func		= bpf_lsm_event_output,
+	.gpl_only       = true,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_CONST_MAP_PTR,
+	.arg2_type      = ARG_ANYTHING,
+	.arg3_type      = ARG_PTR_TO_MEM,
+	.arg4_type      = ARG_CONST_SIZE_OR_ZERO,
+};
+
 static const struct bpf_func_proto *get_bpf_func_proto(enum bpf_func_id
 		func_id, const struct bpf_prog *prog)
 {
@@ -137,6 +156,8 @@  static const struct bpf_func_proto *get_bpf_func_proto(enum bpf_func_id
 		return &bpf_map_lookup_elem_proto;
 	case BPF_FUNC_get_current_pid_tgid:
 		return &bpf_get_current_pid_tgid_proto;
+	case BPF_FUNC_lsm_event_output:
+		return &bpf_lsm_event_output_proto;
 	default:
 		return NULL;
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index fc64ae865526..3511fa271c9b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2823,6 +2823,13 @@  union bpf_attr {
  * 	Return
  * 		On success, the strictly positive length of the string,	including
  * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_lsm_event_output(struct bpf_map *map, u64 flags, void *data, u64 size)
+ *	Description
+ *		This helper is similar to bpf_perf_event_output except that it
+ *		it does not need a context argument.
+ *	Return
+ *		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2940,7 +2947,8 @@  union bpf_attr {
 	FN(probe_read_user),		\
 	FN(probe_read_kernel),		\
 	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(lsm_event_output),		\
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call