netdev - Re: [PATCH net-next v6 1/5] bpf: Add file mode configuration into bpf maps

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 for Android: free password hash cracker in your pocket

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

Message-ID: <59E75C98.5080603@iogearbox.net>
Date:   Wed, 18 Oct 2017 15:52:24 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Chenbo Feng <chenbofeng.kernel@...il.com>, netdev@...r.kernel.org,
        SELinux <Selinux@...ho.nsa.gov>,
        linux-security-module@...r.kernel.org
CC:     Jeffrey Vander Stoep <jeffv@...gle.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        lorenzo@...gle.com, Stephen Smalley <sds@...ho.nsa.gov>,
        James Morris <james.l.morris@...cle.com>,
        Paul Moore <paul@...l-moore.com>,
        Chenbo Feng <fengc@...gle.com>
Subject: Re: [PATCH net-next v6 1/5] bpf: Add file mode configuration into
 bpf maps

Hey Chenbo,

there's still one thing I noticed later one; would have sent a
follow-up, but as you need to respin anyway for the build issue,
here's what is still missing uapi-wise:

On 10/16/2017 09:11 PM, Chenbo Feng wrote:
[...]
> +int bpf_get_file_flag(int flags)
> +{
> +	if ((flags & BPF_F_RDONLY) && (flags & BPF_F_WRONLY))
> +		return -EINVAL;
> +	if (flags & BPF_F_RDONLY)
> +		return O_RDONLY;
> +	if (flags & BPF_F_WRONLY)
> +		return O_WRONLY;
> +	return O_RDWR;
>   }
[...]
> -#define BPF_OBJ_LAST_FIELD bpf_fd
> +#define BPF_OBJ_LAST_FIELD file_flags
>
>   static int bpf_obj_pin(const union bpf_attr *attr)
>   {
> -	if (CHECK_ATTR(BPF_OBJ))
> +	if (CHECK_ATTR(BPF_OBJ) || attr->file_flags != 0)
>   		return -EINVAL;
>
>   	return bpf_obj_pin_user(attr->bpf_fd, u64_to_user_ptr(attr->pathname));
> @@ -1126,7 +1184,8 @@ static int bpf_obj_get(const union bpf_attr *attr)
>   	if (CHECK_ATTR(BPF_OBJ) || attr->bpf_fd != 0)

Here, we also need to check and bail out on ...

attr->file_flags & ~(BPF_F_RDONLY | BPF_F_WRONLY)

... otherwise we cannot extend it with more flags in future. Basically
same principle for mask check you do on map creation, but not yet here.

The same is needed in bpf_map_get_fd_by_id(), too.

The bpf_prog_get_fd_by_id() is covered since BPF_PROG_GET_FD_BY_ID_LAST_FIELD
still points to prog_id, so ok.

>   		return -EINVAL;
> -	return bpf_obj_get_user(u64_to_user_ptr(attr->pathname));
> +	return bpf_obj_get_user(u64_to_user_ptr(attr->pathname),
> +				attr->file_flags);
>   }
>