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  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:   Thu, 20 Aug 2020 23:13:05 -0700
From:   Yonghong Song <yhs@...com>
To:     Udip Pant <udippant@...com>, Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        Andrii Nakryiko <andriin@...com>,
        "David S . Miller" <davem@...emloft.net>
CC:     <netdev@...r.kernel.org>, <bpf@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 bpf 1/2] bpf: verifier: check for packet data access
 based on target prog



On 8/20/20 5:28 PM, Udip Pant wrote:
> While using dynamic program extension (of type BPF_PROG_TYPE_EXT), we
> need to check the program type of the target program to grant the read /
> write access to the packet data.
> 
> The BPF_PROG_TYPE_EXT type can be used to extend types such as XDP, SKB
> and others. Since the BPF_PROG_TYPE_EXT program type on itself is just a
> placeholder for those, we need this extended check for those target
> programs to actually work while using this option.
> 
> Tested this with a freplace xdp program. Without this patch, the
> verifier fails with error 'cannot write into packet'.
> 
> Signed-off-by: Udip Pant <udippant@...com>
> ---
>   kernel/bpf/verifier.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ef938f17b944..4d7604430994 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2629,7 +2629,11 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
>   				       const struct bpf_call_arg_meta *meta,
>   				       enum bpf_access_type t)
>   {
> -	switch (env->prog->type) {
> +	struct bpf_prog *prog = env->prog;
> +	enum bpf_prog_type prog_type = prog->aux->linked_prog ?
> +	      prog->aux->linked_prog->type : prog->type;

I checked the verifier code. There are several places where
prog->type is checked and EXT program type will behave differently
from the linked program.

Maybe abstract the the above logic to one static function like

static enum bpf_prog_type resolved_prog_type(struct bpf_prog *prog)
{
	return prog->aux->linked_prog ? prog->aux->linked_prog->type
				      : prog->type;
}

This function can then be used in different places to give the resolved
prog type.

Besides here checking pkt access permission,
another possible places to consider is return value
in function check_return_code(). Currently,
for EXT program, the result value can be anything. This may need to
be enforced. Could you take a look? It could be others as well.
You can take a look at verifier.c by searching "prog->type".

> +
> +	switch (prog_type) {
>   	/* Program types only with direct read access go here! */
>   	case BPF_PROG_TYPE_LWT_IN:
>   	case BPF_PROG_TYPE_LWT_OUT:
> 

Powered by blists - more mailing lists