* [BUG] lseek on /proc/meminfo is broken in 4.19.59 @ 2019-08-01 6:16 Sergei Turchanov 2019-08-01 7:11 ` Gao Xiang 0 siblings, 1 reply; 10+ messages in thread From: Sergei Turchanov @ 2019-08-01 6:16 UTC (permalink / raw) To: Alexander Viro, linux-fsdevel; +Cc: linux-kernel Hello! (I sent this e-mail two weeks ago with no feedback. Does anyone care? Wrong mailing list? Anything....?) Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. $ ./test /proc/meminfo 0 # Works as expected MemTotal: 394907728 kB MemFree: 173738328 kB ... DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB ----------------------------------------------------------------------- $ ./test 1024 # returns a copy of file after the remainder Will seek to 1024 Data read at offset 1024 gePages: 0 kB ShmemHugePages: 0 kB ShmemPmdMapped: 0 kB HugePages_Total: 0 HugePages_Free: 0 HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB Hugetlb: 0 kB DirectMap4k: 245204 kB DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB MemTotal: 394907728 kB MemFree: 173738328 kB MemAvailable: 379989680 kB Buffers: 355812 kB Cached: 207216224 kB ... DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". Test program: #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> #define SIZE 1024 char buf[SIZE + 1]; int main(int argc, char *argv[]) { int fd; ssize_t rd; off_t ofs = 0; if (argc < 2) { printf("Usage: test <file> [<offset>]\n"); exit(1); } if (-1 == (fd = open(argv[1], O_RDONLY))) { perror("open failed"); exit(1); } if (argc > 2) { ofs = atol(argv[2]); } printf("Will seek to %ld\n", ofs); if (-1 == (lseek(fd, ofs, SEEK_SET))) { perror("lseek failed"); exit(1); } for (;; ofs += rd) { printf("\n\nData read at offset %ld\n", ofs); if (-1 == (rd = read(fd, buf, SIZE))) { perror("read failed"); exit(1); } buf[rd] = '\0'; printf(buf); if (rd < SIZE) { break; } } return 0; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 2019-08-01 6:16 [BUG] lseek on /proc/meminfo is broken in 4.19.59 Sergei Turchanov @ 2019-08-01 7:11 ` Gao Xiang 2019-08-01 7:52 ` Sergei Turchanov 2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov 0 siblings, 2 replies; 10+ messages in thread From: Gao Xiang @ 2019-08-01 7:11 UTC (permalink / raw) To: Sergei Turchanov; +Cc: Alexander Viro, linux-fsdevel, linux-kernel Hi, I just took a glance, maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") I simply reverted it just now and it seems fine... but I haven't digged into this commit. Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and I have no idea whether it's an expected behavior or not... Thanks, Gao Xiang On 2019/8/1 14:16, Sergei Turchanov wrote: > Hello! > > (I sent this e-mail two weeks ago with no feedback. Does anyone care? Wrong mailing list? Anything....?) > > Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. > > Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. > > Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. > > $ ./test /proc/meminfo 0 # Works as expected > > MemTotal: 394907728 kB > MemFree: 173738328 kB > ... > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > > ----------------------------------------------------------------------- > > $ ./test 1024 # returns a copy of file after the remainder > > Will seek to 1024 > > > Data read at offset 1024 > gePages: 0 kB > ShmemHugePages: 0 kB > ShmemPmdMapped: 0 kB > HugePages_Total: 0 > HugePages_Free: 0 > HugePages_Rsvd: 0 > HugePages_Surp: 0 > Hugepagesize: 2048 kB > Hugetlb: 0 kB > DirectMap4k: 245204 kB > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > MemTotal: 394907728 kB > MemFree: 173738328 kB > MemAvailable: 379989680 kB > Buffers: 355812 kB > Cached: 207216224 kB > ... > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > > As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". > > Test program: > > #include <sys/types.h> > #include <sys/stat.h> > #include <unistd.h> > #include <fcntl.h> > #include <stdio.h> > #include <stdlib.h> > > #define SIZE 1024 > char buf[SIZE + 1]; > > int main(int argc, char *argv[]) { > int fd; > ssize_t rd; > off_t ofs = 0; > > if (argc < 2) { > printf("Usage: test <file> [<offset>]\n"); > exit(1); > } > > if (-1 == (fd = open(argv[1], O_RDONLY))) { > perror("open failed"); > exit(1); > } > > if (argc > 2) { > ofs = atol(argv[2]); > } > printf("Will seek to %ld\n", ofs); > > if (-1 == (lseek(fd, ofs, SEEK_SET))) { > perror("lseek failed"); > exit(1); > } > > for (;; ofs += rd) { > printf("\n\nData read at offset %ld\n", ofs); > if (-1 == (rd = read(fd, buf, SIZE))) { > perror("read failed"); > exit(1); > } > buf[rd] = '\0'; > printf(buf); > if (rd < SIZE) { > break; > } > } > > return 0; > } > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 2019-08-01 7:11 ` Gao Xiang @ 2019-08-01 7:52 ` Sergei Turchanov 2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov 1 sibling, 0 replies; 10+ messages in thread From: Sergei Turchanov @ 2019-08-01 7:52 UTC (permalink / raw) To: Gao Xiang; +Cc: Alexander Viro, linux-fsdevel, linux-kernel Hi, Thank you very much for your suggestion. Will certainly do that. With best regards, Sergei. On 01.08.2019 17:11, Gao Xiang wrote: > Hi, > > I just took a glance, maybe due to > commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") > > I simply reverted it just now and it seems fine... but I haven't digged into this commit. > > Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and > I have no idea whether it's an expected behavior or not... > > Thanks, > Gao Xiang > > On 2019/8/1 14:16, Sergei Turchanov wrote: >> Hello! >> >> (I sent this e-mail two weeks ago with no feedback. Does anyone care? Wrong mailing list? Anything....?) >> >> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. >> >> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. >> >> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. >> >> $ ./test /proc/meminfo 0 # Works as expected >> >> MemTotal: 394907728 kB >> MemFree: 173738328 kB >> ... >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> >> ----------------------------------------------------------------------- >> >> $ ./test 1024 # returns a copy of file after the remainder >> >> Will seek to 1024 >> >> >> Data read at offset 1024 >> gePages: 0 kB >> ShmemHugePages: 0 kB >> ShmemPmdMapped: 0 kB >> HugePages_Total: 0 >> HugePages_Free: 0 >> HugePages_Rsvd: 0 >> HugePages_Surp: 0 >> Hugepagesize: 2048 kB >> Hugetlb: 0 kB >> DirectMap4k: 245204 kB >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> MemTotal: 394907728 kB >> MemFree: 173738328 kB >> MemAvailable: 379989680 kB >> Buffers: 355812 kB >> Cached: 207216224 kB >> ... >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> >> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". >> >> Test program: >> >> #include <sys/types.h> >> #include <sys/stat.h> >> #include <unistd.h> >> #include <fcntl.h> >> #include <stdio.h> >> #include <stdlib.h> >> >> #define SIZE 1024 >> char buf[SIZE + 1]; >> >> int main(int argc, char *argv[]) { >> int fd; >> ssize_t rd; >> off_t ofs = 0; >> >> if (argc < 2) { >> printf("Usage: test <file> [<offset>]\n"); >> exit(1); >> } >> >> if (-1 == (fd = open(argv[1], O_RDONLY))) { >> perror("open failed"); >> exit(1); >> } >> >> if (argc > 2) { >> ofs = atol(argv[2]); >> } >> printf("Will seek to %ld\n", ofs); >> >> if (-1 == (lseek(fd, ofs, SEEK_SET))) { >> perror("lseek failed"); >> exit(1); >> } >> >> for (;; ofs += rd) { >> printf("\n\nData read at offset %ld\n", ofs); >> if (-1 == (rd = read(fd, buf, SIZE))) { >> perror("read failed"); >> exit(1); >> } >> buf[rd] = '\0'; >> printf(buf); >> if (rd < SIZE) { >> break; >> } >> } >> >> return 0; >> } >> >> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") 2019-08-01 7:11 ` Gao Xiang 2019-08-01 7:52 ` Sergei Turchanov @ 2019-08-01 8:09 ` Sergei Turchanov 2019-08-01 9:14 ` NeilBrown 1 sibling, 1 reply; 10+ messages in thread From: Sergei Turchanov @ 2019-08-01 8:09 UTC (permalink / raw) To: NeilBrown; +Cc: Alexander Viro, linux-fsdevel, linux-kernel Hello! [ As suggested in previous discussion this behavior may be caused by your commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") ] Original bug report: Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. > On 01.08.2019 17:11, Gao Xiang wrote: > Hi, > > I just took a glance, maybe due to > commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") > > I simply reverted it just now and it seems fine... but I haven't digged into this commit. > > Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and > I have no idea whether it's an expected behavior or not... > > Thanks, > Gao Xiang > > On 2019/8/1 14:16, Sergei Turchanov wrote: $ ./test /proc/meminfo 0 # Works as expected MemTotal: 394907728 kB MemFree: 173738328 kB ... DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB ----------------------------------------------------------------------- $ ./test /proc/meminfo 1024 # returns a copy of file after the remainder Will seek to 1024 Data read at offset 1024 gePages: 0 kB ShmemHugePages: 0 kB ShmemPmdMapped: 0 kB HugePages_Total: 0 HugePages_Free: 0 HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB Hugetlb: 0 kB DirectMap4k: 245204 kB DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB MemTotal: 394907728 kB MemFree: 173738328 kB MemAvailable: 379989680 kB Buffers: 355812 kB Cached: 207216224 kB ... DirectMap2M: 13062144 kB DirectMap1G: 390070272 kB As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". Test program: #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> #define SIZE 1024 char buf[SIZE + 1]; int main(int argc, char *argv[]) { int fd; ssize_t rd; off_t ofs = 0; if (argc < 2) { printf("Usage: test <file> [<offset>]\n"); exit(1); } if (-1 == (fd = open(argv[1], O_RDONLY))) { perror("open failed"); exit(1); } if (argc > 2) { ofs = atol(argv[2]); } printf("Will seek to %ld\n", ofs); if (-1 == (lseek(fd, ofs, SEEK_SET))) { perror("lseek failed"); exit(1); } for (;; ofs += rd) { printf("\n\nData read at offset %ld\n", ofs); if (-1 == (rd = read(fd, buf, SIZE))) { perror("read failed"); exit(1); } buf[rd] = '\0'; printf(buf); if (rd < SIZE) { break; } } return 0; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") 2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov @ 2019-08-01 9:14 ` NeilBrown 2019-08-02 1:40 ` Sergei Turchanov 0 siblings, 1 reply; 10+ messages in thread From: NeilBrown @ 2019-08-01 9:14 UTC (permalink / raw) To: Sergei Turchanov; +Cc: linux-fsdevel, linux-kernel, Alexander Viro [-- Attachment #1: Type: text/plain, Size: 5041 bytes --] On Thu, Aug 01 2019, Sergei Turchanov wrote: > Hello! > > [ > As suggested in previous discussion this behavior may be caused by your > commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") > ] Yes.... I think I can see what happened. removing: - if (!m->count) { - m->from = 0; - m->index++; - } from seq_read meant that ->index didn't get updated in a case that it needs to be. Please confirm that the following patch fixes the problem. I think it is correct, but I need to look it over more carefully in the morning, and see if I can explain why it is correct. Thanks for the report. NeilBrown 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; } > > Original bug report: > > Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. > > Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. > > Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. > > > On 01.08.2019 17:11, Gao Xiang wrote: >> Hi, >> >> I just took a glance, maybe due to >> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") >> >> I simply reverted it just now and it seems fine... but I haven't digged into this commit. >> >> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and >> I have no idea whether it's an expected behavior or not... >> >> Thanks, >> Gao Xiang >> >> On 2019/8/1 14:16, Sergei Turchanov wrote: > > > $ ./test /proc/meminfo 0 # Works as expected > > MemTotal: 394907728 kB > MemFree: 173738328 kB > ... > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > > ----------------------------------------------------------------------- > > $ ./test /proc/meminfo 1024 # returns a copy of file after the remainder > > Will seek to 1024 > > > Data read at offset 1024 > gePages: 0 kB > ShmemHugePages: 0 kB > ShmemPmdMapped: 0 kB > HugePages_Total: 0 > HugePages_Free: 0 > HugePages_Rsvd: 0 > HugePages_Surp: 0 > Hugepagesize: 2048 kB > Hugetlb: 0 kB > DirectMap4k: 245204 kB > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > MemTotal: 394907728 kB > MemFree: 173738328 kB > MemAvailable: 379989680 kB > Buffers: 355812 kB > Cached: 207216224 kB > ... > DirectMap2M: 13062144 kB > DirectMap1G: 390070272 kB > > As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". > > Test program: > > #include <sys/types.h> > #include <sys/stat.h> > #include <unistd.h> > #include <fcntl.h> > #include <stdio.h> > #include <stdlib.h> > > #define SIZE 1024 > char buf[SIZE + 1]; > > int main(int argc, char *argv[]) { > int fd; > ssize_t rd; > off_t ofs = 0; > > if (argc < 2) { > printf("Usage: test <file> [<offset>]\n"); > exit(1); > } > > if (-1 == (fd = open(argv[1], O_RDONLY))) { > perror("open failed"); > exit(1); > } > > if (argc > 2) { > ofs = atol(argv[2]); > } > printf("Will seek to %ld\n", ofs); > > if (-1 == (lseek(fd, ofs, SEEK_SET))) { > perror("lseek failed"); > exit(1); > } > > for (;; ofs += rd) { > printf("\n\nData read at offset %ld\n", ofs); > if (-1 == (rd = read(fd, buf, SIZE))) { > perror("read failed"); > exit(1); > } > buf[rd] = '\0'; > printf(buf); > if (rd < SIZE) { > break; > } > } > > return 0; > } [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") 2019-08-01 9:14 ` NeilBrown @ 2019-08-02 1:40 ` Sergei Turchanov 2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown 0 siblings, 1 reply; 10+ messages in thread From: Sergei Turchanov @ 2019-08-02 1:40 UTC (permalink / raw) To: NeilBrown; +Cc: linux-fsdevel, linux-kernel, Alexander Viro Hello! Yes, your patch fixed this bug. Thank you very much! With best regards, Sergei. On 01.08.2019 19:14, NeilBrown wrote: > On Thu, Aug 01 2019, Sergei Turchanov wrote: > >> Hello! >> >> [ >> As suggested in previous discussion this behavior may be caused by your >> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") >> ] > Yes.... I think I can see what happened. > removing: > - if (!m->count) { > - m->from = 0; > - m->index++; > - } > > from seq_read meant that ->index didn't get updated in a case that it > needs to be. > > Please confirm that the following patch fixes the problem. > I think it is correct, but I need to look it over more carefully in the > morning, and see if I can explain why it is correct. > > Thanks for the report. > NeilBrown > > 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; > } > > >> Original bug report: >> >> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example. >> >> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way. >> >> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x. >> >> > On 01.08.2019 17:11, Gao Xiang wrote: >>> Hi, >>> >>> I just took a glance, maybe due to >>> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") >>> >>> I simply reverted it just now and it seems fine... but I haven't digged into this commit. >>> >>> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and >>> I have no idea whether it's an expected behavior or not... >>> >>> Thanks, >>> Gao Xiang >>> >>> On 2019/8/1 14:16, Sergei Turchanov wrote: >> >> $ ./test /proc/meminfo 0 # Works as expected >> >> MemTotal: 394907728 kB >> MemFree: 173738328 kB >> ... >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> >> ----------------------------------------------------------------------- >> >> $ ./test /proc/meminfo 1024 # returns a copy of file after the remainder >> >> Will seek to 1024 >> >> >> Data read at offset 1024 >> gePages: 0 kB >> ShmemHugePages: 0 kB >> ShmemPmdMapped: 0 kB >> HugePages_Total: 0 >> HugePages_Free: 0 >> HugePages_Rsvd: 0 >> HugePages_Surp: 0 >> Hugepagesize: 2048 kB >> Hugetlb: 0 kB >> DirectMap4k: 245204 kB >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> MemTotal: 394907728 kB >> MemFree: 173738328 kB >> MemAvailable: 379989680 kB >> Buffers: 355812 kB >> Cached: 207216224 kB >> ... >> DirectMap2M: 13062144 kB >> DirectMap1G: 390070272 kB >> >> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read". >> >> Test program: >> >> #include <sys/types.h> >> #include <sys/stat.h> >> #include <unistd.h> >> #include <fcntl.h> >> #include <stdio.h> >> #include <stdlib.h> >> >> #define SIZE 1024 >> char buf[SIZE + 1]; >> >> int main(int argc, char *argv[]) { >> int fd; >> ssize_t rd; >> off_t ofs = 0; >> >> if (argc < 2) { >> printf("Usage: test <file> [<offset>]\n"); >> exit(1); >> } >> >> if (-1 == (fd = open(argv[1], O_RDONLY))) { >> perror("open failed"); >> exit(1); >> } >> >> if (argc > 2) { >> ofs = atol(argv[2]); >> } >> printf("Will seek to %ld\n", ofs); >> >> if (-1 == (lseek(fd, ofs, SEEK_SET))) { >> perror("lseek failed"); >> exit(1); >> } >> >> for (;; ofs += rd) { >> printf("\n\nData read at offset %ld\n", ofs); >> if (-1 == (rd = read(fd, buf, SIZE))) { >> perror("read failed"); >> exit(1); >> } >> buf[rd] = '\0'; >> printf(buf); >> if (rd < SIZE) { >> break; >> } >> } >> >> return 0; >> } ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] seq_file: fix problem when seeking mid-record. 2019-08-02 1:40 ` Sergei Turchanov @ 2019-08-05 4:26 ` NeilBrown 2019-08-05 9:15 ` Markus Elfring 2019-08-06 22:42 ` [PATCH] " Andrew Morton 0 siblings, 2 replies; 10+ messages in thread From: NeilBrown @ 2019-08-05 4:26 UTC (permalink / raw) To: Andrew Morton, Alexander Viro, Sergei Turchanov Cc: linux-fsdevel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1817 bytes --] 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(-) 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; } -- 2.14.0.rc0.dirty [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: seq_file: fix problem when seeking mid-record. 2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown @ 2019-08-05 9:15 ` Markus Elfring 2019-08-05 10:08 ` NeilBrown 2019-08-06 22:42 ` [PATCH] " Andrew Morton 1 sibling, 1 reply; 10+ messages in thread From: Markus Elfring @ 2019-08-05 9:15 UTC (permalink / raw) To: Neil Brown, Andrew Morton, Alexander Viro Cc: linux-fsdevel, linux-kernel, Sergei Turchanov > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: seq_file: fix problem when seeking mid-record. 2019-08-05 9:15 ` Markus Elfring @ 2019-08-05 10:08 ` NeilBrown 0 siblings, 0 replies; 10+ messages in thread From: NeilBrown @ 2019-08-05 10:08 UTC (permalink / raw) To: Markus Elfring, Andrew Morton, Alexander Viro Cc: Sergei Turchanov, linux-fsdevel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 547 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] seq_file: fix problem when seeking mid-record. 2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown 2019-08-05 9:15 ` Markus Elfring @ 2019-08-06 22:42 ` Andrew Morton 1 sibling, 0 replies; 10+ messages in thread From: Andrew Morton @ 2019-08-06 22:42 UTC (permalink / raw) To: NeilBrown; +Cc: Alexander Viro, Sergei Turchanov, linux-fsdevel, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-08-06 22:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-01 6:16 [BUG] lseek on /proc/meminfo is broken in 4.19.59 Sergei Turchanov 2019-08-01 7:11 ` Gao Xiang 2019-08-01 7:52 ` Sergei Turchanov 2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov 2019-08-01 9:14 ` NeilBrown 2019-08-02 1:40 ` Sergei Turchanov 2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown 2019-08-05 9:15 ` Markus Elfring 2019-08-05 10:08 ` NeilBrown 2019-08-06 22:42 ` [PATCH] " Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).