netdev - Re: [PATCH bpf-next v13 1/2] bpf: add new helper get_fd_path for mapping a file descriptor to a pathname

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]

Message-ID: <a8a763e2-65d2-7c71-e99d-ffae1523f0f0@iogearbox.net>
Date:   Tue, 17 Dec 2019 20:39:29 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Yonghong Song <yhs@...com>, Wenbo Zhang <ethercflow@...il.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Cc:     "ast@...nel.org" <ast@...nel.org>,
        "bgregg@...flix.com" <bgregg@...flix.com>,
        "andrii.nakryiko@...il.com" <andrii.nakryiko@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v13 1/2] bpf: add new helper get_fd_path for
 mapping a file descriptor to a pathname

On 12/17/19 5:29 PM, Yonghong Song wrote:
> On 12/17/19 1:47 AM, Wenbo Zhang wrote:
[...]
>> + *		On failure, it is filled with zeroes.
[...]
>>     */
>>    #define __BPF_FUNC_MAPPER(FN)		\
>>    	FN(unspec),			\
>> @@ -2938,7 +2964,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(get_fd_path),
>>    
>>    /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>>     * function eBPF program intends to call
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index e5ef4ae9edb5..43a6aa6ad967 100644
>> --- a/kernel/trace/bpf_trace.c
>> +++ b/kernel/trace/bpf_trace.c
>> @@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
>>    	.arg1_type	= ARG_ANYTHING,
>>    };
>>    
>> +BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
>> +{
>> +	int ret = -EBADF;
>> +	struct file *f;
>> +	char *p;
>> +
>> +	/* Ensure we're in user context which is safe for the helper to
>> +	 * run. This helper has no business in a kthread.
>> +	 */
>> +	if (unlikely(in_interrupt() ||
>> +		     current->flags & (PF_KTHREAD | PF_EXITING))) {
>> +		ret = -EPERM;
>> +		goto error;
>> +	}
>> +
>> +	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
>> +	 * have any sleepable code, so it's ok to be here.
>> +	 */
>> +	f = fget_raw(fd);
>> +	if (!f)
>> +		goto error;
>> +
>> +	/* For unmountable pseudo filesystem, it seems to have no meaning
>> +	 * to get their fake paths as they don't have path, and to be no
>> +	 * way to validate this function pointer can be always safe to call
>> +	 * in the current context.
>> +	 */
>> +	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
>> +		ret = -EINVAL;
>> +		fput(f);
>> +		goto error;
>> +	}
>> +
>> +	/* After filter unmountable pseudo filesytem, d_path won't call
>> +	 * dentry->d_op->d_name(), the normally path doesn't have any
>> +	 * sleepable code, and despite it uses the current macro to get
>> +	 * fs_struct (current->fs), we've already ensured we're in user
>> +	 * context, so it's ok to be here.
>> +	 */
>> +	p = d_path(&f->f_path, dst, size);
>> +	if (IS_ERR(p)) {
>> +		ret = PTR_ERR(p);
>> +		fput(f);
>> +		goto error;
>> +	}
>> +
>> +	ret = strlen(p) + 1;
>> +	memmove(dst, p, ret);
>> +	fput(f);
>> +	return ret;
>> +
>> +error:
>> +	memset(dst, '0', size);

You fill it with 0x30's ...

>> +	return ret;
>> +}