netdev - RE: [PATCH bpf-next 1/3] bpf: enable BPF_PROG_TEST_RUN for raw

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: <5f6acdd4ef8a2_3657820819@john-XPS-13-9370.notmuch>
Date:   Tue, 22 Sep 2020 21:23:48 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Song Liu <songliubraving@...com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Cc:     kernel-team@...com, ast@...nel.org, daniel@...earbox.net,
        john.fastabend@...il.com, kpsingh@...omium.org,
        Song Liu <songliubraving@...com>
Subject: RE: [PATCH bpf-next 1/3] bpf: enable BPF_PROG_TEST_RUN for
 raw_tracepoint

Song Liu wrote:
> Add .test_run for raw_tracepoint. Also, introduce a new feature that runs
> the target program on a specific CPU. This is achieved by a new flag in
> bpf_attr.test, cpu_plus. For compatibility, cpu_plus == 0 means run the
> program on current cpu, cpu_plus > 0 means run the program on cpu with id
> (cpu_plus - 1). This feature is needed for BPF programs that handle
> perf_event and other percpu resources, as the program can access these
> resource locally.
> 
> Signed-off-by: Song Liu <songliubraving@...com>
> ---

Acked-by: John Fastabend <john.fastabend@...il.com>

[...]

> +
> +int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
> +			     const union bpf_attr *kattr,
> +			     union bpf_attr __user *uattr)
> +{
> +	void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> +	__u32 ctx_size_in = kattr->test.ctx_size_in;
> +	struct bpf_raw_tp_test_run_info info;
> +	int cpu, err = 0;
> +
> +	/* doesn't support data_in/out, ctx_out, duration, or repeat */
> +	if (kattr->test.data_in || kattr->test.data_out ||
> +	    kattr->test.ctx_out || kattr->test.duration ||
> +	    kattr->test.repeat)
> +		return -EINVAL;
> +
> +	if (ctx_size_in < prog->aux->max_ctx_offset)
> +		return -EINVAL;
> +
> +	if (ctx_size_in) {
> +		info.ctx = kzalloc(ctx_size_in, GFP_USER);
> +		if (!info.ctx)
> +			return -ENOMEM;
> +		if (copy_from_user(info.ctx, ctx_in, ctx_size_in)) {
> +			err = -EFAULT;
> +			goto out;
> +		}
> +	} else {
> +		info.ctx = NULL;
> +	}
> +
> +	info.prog = prog;
> +	cpu = kattr->test.cpu_plus - 1;
> +
> +	if (!kattr->test.cpu_plus || cpu == smp_processor_id()) {
> +		__bpf_prog_test_run_raw_tp(&info);
> +	} else {
> +		/* smp_call_function_single() also checks cpu_online()
> +		 * after csd_lock(). However, since cpu_plus is from user
> +		 * space, let's do an extra quick check to filter out
> +		 * invalid value before smp_call_function_single().
> +		 */
> +		if (!cpu_online(cpu)) {
> +			err = -ENXIO;
> +			goto out;
> +		}
> +
> +		err = smp_call_function_single(cpu, __bpf_prog_test_run_raw_tp,
> +					       &info, 1);
> +		if (err)
> +			goto out;
> +	}
> +
> +	if (copy_to_user(&uattr->test.retval, &info.retval, sizeof(u32))) {
> +		err = -EFAULT;
> +		goto out;
> +	}

This goto is not needed. I don't mind it though.

> +
> +out:
> +	kfree(info.ctx);
> +	return err;
> +}
> +