| Message ID | 87mugojl0f.fsf@notabene.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | seq_file: fix problem when seeking mid-record. | expand |
> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code > and interface") Please do not split this tag across multiple lines in the final commit description. Regards, Markus
On Mon, Aug 05 2019, Markus Elfring wrote: >> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code >> and interface") > > Please do not split this tag across multiple lines in the final commit description. I tend to agree... I had previously seen "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" warnings from checkpatch, but one closer look that doesn't apply to Fixes: lines (among other special cases). Maybe Andrew will fix it up for me when it applies .... (please!) Thanks, NeilBrown
On Mon, 05 Aug 2019 14:26:08 +1000 NeilBrown <neilb@suse.com> wrote: > If you use lseek or similar (e.g. pread) to access > a location in a seq_file file that is within a record, > rather than at a record boundary, then the first read > will return the remainder of the record, and the second > read will return the whole of that same record (instead > of the next record). > Whnn seeking to a record boundary, the next record is > correctly returned. ouch. I'm surprised it took this long to be noticed. Maybe we need a seqfile-basher in tools/testing/selftests/proc or somewhere.
diff --git a/fs/seq_file.c b/fs/seq_file.c index 04f09689cd6d..1600034a929b 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -119,6 +119,7 @@ static int traverse(struct seq_file *m, loff_t offset) } if (seq_has_overflowed(m)) goto Eoverflow; + p = m->op->next(m, p, &m->index); if (pos + m->count > offset) { m->from = offset - pos; m->count -= m->from; @@ -126,7 +127,6 @@ static int traverse(struct seq_file *m, loff_t offset) } pos += m->count; m->count = 0; - p = m->op->next(m, p, &m->index); if (pos == offset) break; }
If you use lseek or similar (e.g. pread) to access a location in a seq_file file that is within a record, rather than at a record boundary, then the first read will return the remainder of the record, and the second read will return the whole of that same record (instead of the next record). Whnn seeking to a record boundary, the next record is correctly returned. This bug was introduced by a recent patch (identified below) Before that patch, seq_read() would increment m->index when the last of the buffer was returned (m->count == 0). After that patch, we rely on ->next to increment m->index after filling the buffer - but there was one place where that didn't happen. Link: https://lkml.kernel.org/lkml/877e7xl029.fsf@notabene.neil.brown.name/ Reported-by-tested-by: Sergei Turchanov <turchanov@farpost.com> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Cc: stable@vger.kernel.org (v4.19+) Signed-off-by: NeilBrown <neilb@suse.com> --- Hi Andrew: as you applied the offending patch for me, maybe you could queue up this fix too. Thanks, NeilBrown fs/seq_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)