Re: [PATCH 1/1] map_seq_next should increase position index

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

On 1/24/20 7:17 AM, Vasily Averin wrote:
> if seq_file .next fuction does not change position index,
> read after some lseek can generate unexpected output.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=206283
> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>
> ---
>   kernel/bpf/inode.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> index ecf42be..9008a20 100644
> --- a/kernel/bpf/inode.c
> +++ b/kernel/bpf/inode.c
> @@ -196,6 +196,7 @@ static void *map_seq_next(struct seq_file *m, void *v, loff_t *pos)
>   	void *key = map_iter(m)->key;
>   	void *prev_key;
>   
> +	(*pos)++;
>   	if (map_iter(m)->done)
>   		return NULL;
>   

Hm, how did you test this change? Please elaborate, since in map_seq_next()
we do increment position index:

static void *map_seq_next(struct seq_file *m, void *v, loff_t *pos)
{
        struct bpf_map *map = seq_file_to_map(m);
        void *key = map_iter(m)->key;
        void *prev_key;

        if (map_iter(m)->done)
                return NULL;

        if (unlikely(v == SEQ_START_TOKEN))
                prev_key = NULL;
        else
                prev_key = key;

        if (map->ops->map_get_next_key(map, prev_key, key)) {
                map_iter(m)->done = true;
                return NULL;
        }

        ++(*pos);                    <------------ here
        return key;
}

With your change we'd increment twice. What is missing here?

Thanks,
Daniel