linux-kernel - Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 4 Sep 2015 13:27:05 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Tycho Andersen <tycho.andersen@...onical.com>
Cc:	Kees Cook <keescook@...omium.org>,
	Alexei Starovoitov <ast@...nel.org>,
	Will Drewry <wad@...omium.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Andy Lutomirski <luto@...capital.net>,
	Pavel Emelyanov <xemul@...allels.com>,
	"Serge E. Hallyn" <serge.hallyn@...ntu.com>,
	Daniel Borkmann <daniel@...earbox.net>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program

On Fri, Sep 04, 2015 at 10:04:21AM -0600, Tycho Andersen wrote:
> This commit adds a way to dump eBPF programs. The initial implementation
> doesn't support maps, and therefore only allows dumping seccomp ebpf
> programs which themselves don't currently support maps.
> 
> 
> Signed-off-by: Tycho Andersen <tycho.andersen@...onical.com>
> CC: Kees Cook <keescook@...omium.org>
> CC: Will Drewry <wad@...omium.org>
> CC: Oleg Nesterov <oleg@...hat.com>
> CC: Andy Lutomirski <luto@...capital.net>
> CC: Pavel Emelyanov <xemul@...allels.com>
> CC: Serge E. Hallyn <serge.hallyn@...ntu.com>
> CC: Alexei Starovoitov <ast@...nel.org>
> CC: Daniel Borkmann <daniel@...earbox.net>
> ---
>  include/uapi/linux/bpf.h | 15 +++++++++++++++
>  kernel/bpf/syscall.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 79b825a..c5d8dc2 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -107,6 +107,13 @@ enum bpf_cmd {
>  	 * returns fd or negative error
>  	 */
>  	BPF_PROG_LOAD,
> +
> +	/* dump an existing bpf
> +	 * err = bpf(BPF_PROG_DUMP, union bpf_attr *attr, u32 size)
> +	 * Using attr->prog_fd, attr->dump_insn_cnt, attr->dump_insns
> +	 * returns zero or negative error
> +	 */
> +	BPF_PROG_DUMP,
>  };
>  
>  enum bpf_map_type {
> @@ -160,6 +167,14 @@ union bpf_attr {
>  		__aligned_u64	log_buf;	/* user supplied buffer */
>  		__u32		kern_version;	/* checked when prog_type=kprobe */
>  	};
> +
> +	struct { /* anonymous struct used by BPF_PROG_DUMP command */
> +		__u32		prog_fd;
> +		__u32		dump_insn_cnt;
> +		__aligned_u64	dump_insns;	/* user supplied buffer */
> +		__u8		gpl_compatible;
> +		__u64		prog_id;	/* unique id for this prog */
> +	};

my first reaction was to may be reuse existing struct used to load,
but I guess it's actually cleaner to have a new one like you did.
though prog_fd looks redundant and prog_id is ...

> +	if (put_user((u64) prog, &uattr->prog_id))
> +		goto out;

.. is definitely not secure.

> We export the GPL bit as well as a unique ID for the program so that
> userspace can detect when two seccomp filters were inherited from each
> other and clone the filter tree accordingly.

you mean that in-kernel prog pointer is the same?
I think user space can memcmp insns of programs instead?
Are you trying to solve the case when parent has an FD for bpf program
and child has another FD that points to the same program, and both
doing dump and need to coordinate?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/