On 1/24/20 1:03 PM, Daniel Borkmann wrote: > 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: Position index should be updated even if .next returns NULL. Sorry, I've not tested this case. $ dd if=AFFECTED_FILE bs=1000 skip=1 With any huge bs it will generate last line. If you'll specify bs in middle of last line -- dd will output rest of list line and then whole last line once again. > 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 You are right, I've missed it, it should be removed. > return key; > } > > With your change we'd increment twice. What is missing here?
