netdev - Re: [PATCH bpf-next v4 21/21] tools/bpf: selftests: add bpf

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: <3b2b2e40-4e80-2a5d-e479-fc12a95162f2@fb.com>
Date:   Sat, 9 May 2020 22:01:35 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Andrii Nakryiko <andriin@...com>, <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>, <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>, <kernel-team@...com>
Subject: Re: [PATCH bpf-next v4 21/21] tools/bpf: selftests: add bpf_iter
 selftests



On 5/9/20 5:34 PM, Alexei Starovoitov wrote:
> On Sat, May 09, 2020 at 10:59:23AM -0700, Yonghong Song wrote:
>> +static volatile const __u32 ret1;
>> +
>> +SEC("iter/bpf_map")
>> +int dump_bpf_map(struct bpf_iter__bpf_map *ctx)
>> +{
>> +	struct seq_file *seq = ctx->meta->seq;
>> +	struct bpf_map *map = ctx->map;
>> +	__u64 seq_num;
>> +	int i, ret = 0;
>> +
>> +	if (map == (void *)0)
>> +		return 0;
>> +
>> +	/* only dump map1_id and map2_id */
>> +	if (map->id != map1_id && map->id != map2_id)
>> +		return 0;
>> +
>> +	seq_num = ctx->meta->seq_num;
>> +	if (map->id == map1_id) {
>> +		map1_seqnum = seq_num;
>> +		map1_accessed++;
>> +	}
>> +
>> +	if (map->id == map2_id) {
>> +		if (map2_accessed == 0) {
>> +			map2_seqnum1 = seq_num;
>> +			if (ret1)
>> +				ret = 1;
>> +		} else {
>> +			map2_seqnum2 = seq_num;
>> +		}
>> +		map2_accessed++;
>> +	}
>> +
>> +	/* fill seq_file buffer */
>> +	for (i = 0; i < print_len; i++)
>> +		bpf_seq_write(seq, &seq_num, sizeof(seq_num));
>> +
>> +	return ret;
>> +}
> 
> I couldn't find where 'return 1' behavior is documented clearly.

It is in the commit comments:

commit 15d83c4d7cef5c067a8b075ce59e97df4f60706e
Author: Yonghong Song <yhs@...com>
Date:   Sat May 9 10:59:00 2020 -0700

     bpf: Allow loading of a bpf_iter program
...
     The program return value must be 0 or 1 for now.
       0 : successful, except potential seq_file buffer overflow
           which is handled by seq_file reader.
       1 : request to restart the same object

Internally, bpf program returning 1 will translate
show() return -EAGAIN and this error code will
return to user space.

I will add some comments in the code to
document this behavior.

> I think it's a workaround for overflow.

This can be used for overflow but overflow already been taken
care of by bpf_seq_read(). This is mostly used for other use
cases:
    - currently under RT-linux, bpf_seq_printf() may return
      -EBUSY. In this case, bpf program itself can request
      retrying the same object.
    - for other conditions where bpf program itself wants
      to retry the same object. For example, hash table full,
      the bpf progam can return 1, in which case, user space
      read() will receive -EAGAIN and may check and make room
      for hash table and then read() again.

> When bpf prog detects overflow it can request replay of the element?

It can. But it can return 0 too since bpf_seq_read() handles
this transparently.

> What if it keeps returning 1 ? read() will never finish?

The read() will finish and return -EAGAIN to user space.
It is up to user space to decide whether to call read()
again or not.