netdev - Re: [PATCH v5 perf,bpf 07/15] perf, bpf: save bpf_prog

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:   Mon, 4 Mar 2019 20:31:45 +0000
From:   Song Liu <songliubraving@...com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     Networking <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "acme@...hat.com" <acme@...hat.com>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "namhyung@...nel.org" <namhyung@...nel.org>
Subject: Re: [PATCH v5 perf,bpf 07/15] perf, bpf: save bpf_prog_info
 information as headers to perf.data



> On Mar 4, 2019, at 12:23 PM, Jiri Olsa <jolsa@...hat.com> wrote:
> 
> On Mon, Mar 04, 2019 at 07:36:14PM +0000, Song Liu wrote:
>> 
>> 
>>> On Mar 4, 2019, at 5:52 AM, Jiri Olsa <jolsa@...hat.com> wrote:
>>> 
>>> On Wed, Feb 27, 2019 at 09:06:35PM -0800, Song Liu wrote:
>>> 
>>> SNIP
>>> 
>>>> +static int process_bpf_prog_info(struct feat_fd *ff,
>>>> +				 void *data __maybe_unused)
>>>> +{
>>>> +	struct bpf_prog_info_linear *info_linear;
>>>> +	struct bpf_prog_info_node *info_node;
>>>> +	struct perf_env *env = &ff->ph->env;
>>>> +	u32 count, i;
>>>> +	int err = -1;
>>>> +
>>>> +	if (do_read_u32(ff, &count))
>>>> +		return -1;
>>>> +
>>>> +	down_write(&env->bpf_progs.lock);
>>>> +
>>>> +	for (i = 0; i < count; ++i) {
>>>> +		u32 info_len, data_len;
>>>> +
>>>> +		info_linear = NULL;
>>>> +		info_node = NULL;
>>>> +		if (do_read_u32(ff, &info_len))
>>>> +			goto out;
>>>> +		if (do_read_u32(ff, &data_len))
>>>> +			goto out;
>>>> +
>>>> +		if (info_len > sizeof(struct bpf_prog_info)) {
>>>> +			pr_warning("detected invalid bpf_prog_info\n");
>>>> +			goto out;
>>>> +		}
>>>> +
>>>> +		info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
>>>> +				     data_len);
>>>> +		if (!info_linear)
>>>> +			goto out;
>>>> +		info_linear->info_len = sizeof(struct bpf_prog_info);
>>>> +		info_linear->data_len = data_len;
>>>> +		if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
>>>> +			goto out;
>>>> +		if (__do_read(ff, &info_linear->info, info_len))
>>>> +			goto out;
>>>> +		if (info_len < sizeof(struct bpf_prog_info))
>>>> +			memset(((void *)(&info_linear->info)) + info_len, 0,
>>>> +			       sizeof(struct bpf_prog_info) - info_len);
>>>> +
>>>> +		if (__do_read(ff, info_linear->data, data_len))
>>>> +			goto out;
>>>> +
>>>> +		/* endian mismatch, drop the info, continue */
>>>> +		if (ff->ph->needs_swap) {
>>>> +			free(info_linear);
>>>> +			continue;
>>>> +		}
>>> 
>>> so in this case we can check for needs_swap in the begining
>>> of the function and bail out without reading all the data
>> 
>> If we bail out, perf-record will fail. If we read all the data 
>> but ignore them, perf-record will continue without bpf annotation 
>> support. I think the latter is a better experience with little 
>> effort here. 
> 
> i did not mean bail out with error, just return 0, warn and go on
> 
> jirka

In this case, do we need to read and discard all the data? If not, 
the next header type (or data) will interpret these data as 
something else, right? Or did I miss some protection against such
cases?

Thanks,
Song

> 
>> 
>>> 
>>> also please display soem error message saying we don't support
>>> ebpf progs data report over the different endianity
>> 
>> I will add a warning in the next version. 
>> 
>> Thanks,
>> Song
>>